lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 May 2016 19:41:29 +0200 From: Matias Bjørling <mb@...htnvm.io> To: Greg KH <gregkh@...uxfoundation.org> Cc: "Simon A. F. Lund" <slund@...xlabs.com>, linux-block@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [RFC PATCH] lightnvm: expose configuration through sysfs 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.
Powered by blists - more mailing lists