[PATCH V8 1/1] nvme: allow passthru cmd error logging

[Chaitanya Kulkarni]

Sagi,

>> nvme_mpath_add_disk(ns, info->anagrpid);
>>           nvme_fault_inject_init(&ns->fault_inject, 
>> ns->disk->disk_name);
>> +
>> +       /*
>> +        * Set ns->disk->device->driver_data to ns so we can access
>> +        * ns->logging_enabled in 
>> nvme_passthru_err_log_enabled_store() and
>> +        * nvme_passthru_err_log_enabled_show().
>> +        */
>> +       dev_set_drvdata(disk_to_dev(ns->disk), ns);
>
> Is this needed?

Yes see explanation below ..

[...]

>
>>
>>    static ssize_t nvme_passthru_err_log_enabled_store(struct device 
>> *dev,
>>                   struct device_attribute *attr, const char *buf, size_t
>> count)
>>    {
>> -       int err;
>>           bool passthru_err_log_enabled;
>> +       int err;
>>
>>           err = kstrtobool(buf, &passthru_err_log_enabled);
>>           if (err)
>>                   return -EINVAL;
>>
>> -       dev->logging_enabled = passthru_err_log_enabled;
>> +       if (is_nvme_class(dev->class)) {
>> +               struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>> +
>> +               ctrl->logging_enabled = passthru_err_log_enabled;
>> +       } else {
>> +               struct nvme_ns *ns = dev_get_drvdata(dev);
>> +
>> +               ns->logging_enabled = passthru_err_log_enabled;
>> +       }
>
> Why mix ctrl with ns sysfs handlers?

sorry I didn't understand your question clearly ...

In the original implementation we get two different struct device objects
for following commands in sysfs, that sets struct device:logging_enabled 
flag,
which is also used to determine the logging in nvme_init_request() :-

echo 1 > /sys/class/nvme/nvme0/passthru_err_log_enabled
echo 1 > /sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled

nvme_init_ctrl() already sets dev_set_drvdata(ctrl->device, ctrl). In this
proposal above change in nvme_alloc_ns() sets the
dev_set_drvdata(disk_to_dev(ns->disk), ns).

This allows us to get the ctrl or ns object associated with the struct 
device
we get in the sysfs, then based on the device class we update 
logging_enabled
flags for either ctrl or ns respectively. In nvme_init_request() I use
ctrl->logging_enabled and ns->logging_enabled based on admin or io cmd.

-ck