lkml.org 
[lkml]   [2016]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] lightnvm: expose configuration through sysfs
From
Date


On 05/06/2016 06:37 PM, Greg KH wrote:
> On Wed, Apr 27, 2016 at 09:26:39PM +0200, Matias Bjørling wrote:
>> On 04/27/2016 09:00 PM, Greg KH wrote:
>>> On Wed, Apr 27, 2016 at 08:20:33PM +0200, Matias Bjørling wrote:
>>>>
>>>>
>>>> On 04/27/2016 07:41 PM, Greg KH wrote:
>>>>> On Wed, Apr 27, 2016 at 10:18:57AM -0700, Simon A. F. Lund wrote:
>>>>>> --- a/include/linux/lightnvm.h
>>>>>> +++ b/include/linux/lightnvm.h
>>>>>> @@ -174,6 +174,7 @@ struct nvm_id_group {
>>>>>> u16 cpar;
>>>>>>
>>>>>> struct nvm_id_lp_tbl lptbl;
>>>>>> + struct kobject kobj;
>>>>>> };
>>>>>>
>>>>>> struct nvm_addr_format {
>>>>>> @@ -205,6 +206,7 @@ struct nvm_target {
>>>>>> struct list_head list;
>>>>>> struct nvm_tgt_type *type;
>>>>>> struct gendisk *disk;
>>>>>> + struct kobject kobj;
>>>>>> };
>>>>>>
>>>>>> struct nvm_tgt_instance {
>>>>>> @@ -360,6 +362,8 @@ struct nvm_dev {
>>>>>>
>>>>>> struct mutex mlock;
>>>>>> spinlock_t lock;
>>>>>> +
>>>>>> + struct kobject kobj;
>>>>>> };
>>>>>>
>>>>>> static inline struct ppa_addr generic_to_dev_addr(struct nvm_dev *dev,
>>>>>
>>>>> Never use "raw" kobjects in a driver for a device. You just guaranteed
>>>>> that userspace tools will not see these devices or attributes, which
>>>>> implies you didn't really test this using libudev :(
>>>>>
>>>>> Please use real devices, attached to the real devices your disks already
>>>>> have in the tree.
>>>>>
>>>>> And are you sure you didn't just mess up your reference counting by
>>>>> now having the lifecycle of these structures be dictated by the kobject?
>>>>>
>>>>> thanks,
>>>>>
>>>>> greg k-h
>>>>>
>>>>
>>>> Hi Greg,
>>>>
>>>> Thanks for the feedback.
>>>>
>>>> lightnvm doesn't have anything to hook up with in the /dev/block/* until a
>>>> device is exposed through a target. A device goes into a staging area, and
>>>> then later is configured to expose a block device.
>>>>
>>>> In the case of NVMe device driver, the driver brings up a device, identifies
>>>> it as a lightnvm device, then calls nvm_register and registers the device.
>>>> It skips the registration as a block device.
>>>
>>> But you could register it with sysfs at this point in time, giving you
>>> a place in the device tree. Which would be good.
>>
>> As an example, when the device is identified by the nvme device driver, the
>> nvm_register() registers the device (e.g. nvme0n1) in sysfs and places it
>> here until further configuration:
>>
>> /sys/devices/virtual/misc/lightnvm/devices/nvme0n1
>>
>> It would expose a representation of the lightnvm configuration
>>
>> Then when targets are added, we would put the target (e.g. tgt0) in
>>
>> /sys/devices/virtual/misc/lightnvm/targets/tgt0
>>
>> and that one could reference the device by
>>
>> /sys/devices/virtual/misc/lightnvm/targets/tgt0/devices/nvme0n1
>>
>> pointing to
>>
>> /sys/devices/virtual/misc/lightnvm/devices/nvme0n1
>>
>> A target can span multiple devices (that's why the targets/*/devices link is
>> there)
>>
>> Does that make sense? and in this case, the raw kobjects make sense to use,
>> as we don't have anything to bind them up to, other than the misc device we
>> registered.
>
> Maybe, but really, why not use a struct device? You just prevented all
> userspace tools from finding your devices (i.e. libudev), so you are
> going to have a hard time with any tools that want to see the tree of
> devices in the system.
>
> And you also just lost the power management chain as well, the driver
> model doesn't know to drop into a kobject, as it can't, so you might not
> be working properly at all with that here for suspend/resume and the
> like.
>
> good luck,
>
> greg k-h
>

Thanks greg. We are reworking the struct device into it. Requires a
little bit more work from the nvme device driver, else it should be
good. Thanks for your feedback.

\
 
 \ /
  Last update: 2016-05-06 20:01    [W:0.423 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site