Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-13 Thread Logan Gunthorpe
Hey, I like the interface. It's just Greg that needs to comment on whether using the kobj.parent for this purpose is actually sane. That was his argument from the beginning. Logan On 13/02/17 01:47 PM, Dan Williams wrote: > On Sat, Feb 11, 2017 at 9:42 PM, Logan Gunthorpe wrote: >> On 11/02/17

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-13 Thread Dan Williams
On Sat, Feb 11, 2017 at 9:42 PM, Logan Gunthorpe wrote: > On 11/02/17 11:58 AM, Dan Williams wrote: >> Also when using an embedded cdev how would you recommend avoiding this >> problem? > > I don't know. Hopefully, Greg has a good idea. But it sounds like a > general problem that a lot of cdev's

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Logan Gunthorpe
On 11/02/17 11:58 AM, Dan Williams wrote: > Also when using an embedded cdev how would you recommend avoiding this > problem? I don't know. Hopefully, Greg has a good idea. But it sounds like a general problem that a lot of cdev's actually suffer from without realizing. Perhaps we need a more gen

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Dan Williams
On Sat, Feb 11, 2017 at 10:55 AM, Dan Williams wrote: > On Sat, Feb 11, 2017 at 10:43 AM, Logan Gunthorpe wrote: >> >> On 11/02/17 11:27 AM, Dan Williams wrote: >>> Why, when the lifetime of the cdev is already correct? >> >> Well, it's only correct if you use the kobj parent trick which Greg is

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Dan Williams
On Sat, Feb 11, 2017 at 10:43 AM, Logan Gunthorpe wrote: > > On 11/02/17 11:27 AM, Dan Williams wrote: >> Why, when the lifetime of the cdev is already correct? > > Well, it's only correct if you use the kobj parent trick which Greg is > arguing against. As someone reviewing/copying code that tric

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Logan Gunthorpe
On 11/02/17 11:27 AM, Dan Williams wrote: > Why, when the lifetime of the cdev is already correct? Well, it's only correct if you use the kobj parent trick which Greg is arguing against. As someone reviewing/copying code that trick is unclear, undocumented and it looks rather odd messing with int

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Dan Williams
On Sat, Feb 11, 2017 at 9:59 AM, Logan Gunthorpe wrote: > On 11/02/17 01:56 AM, Dan Williams wrote: >> >> When the device is unregistered it invalidates all existing mappings, >> but the driver may continue to service vm fault requests until the >> final put of the cdev. Until that time the fault

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Logan Gunthorpe
On 11/02/17 01:56 AM, Dan Williams wrote: When the device is unregistered it invalidates all existing mappings, but the driver may continue to service vm fault requests until the final put of the cdev. Until that time the fault handler needs to be able to check dax_dev->alive. Since the final cde

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-11 Thread Dan Williams
On Fri, Feb 10, 2017 at 11:16 PM, Greg Kroah-Hartman wrote: > On Fri, Feb 10, 2017 at 02:25:35PM -0800, Dan Williams wrote: >> On Fri, Feb 10, 2017 at 12:17 PM, Greg Kroah-Hartman >> wrote: >> > On Fri, Feb 10, 2017 at 11:41:20AM -0800, Dan Williams wrote: >> >> On Fri, Feb 10, 2017 at 11:19 AM,

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Greg Kroah-Hartman
On Fri, Feb 10, 2017 at 02:25:35PM -0800, Dan Williams wrote: > On Fri, Feb 10, 2017 at 12:17 PM, Greg Kroah-Hartman > wrote: > > On Fri, Feb 10, 2017 at 11:41:20AM -0800, Dan Williams wrote: > >> On Fri, Feb 10, 2017 at 11:19 AM, Logan Gunthorpe > >> wrote: > >> > I copied this code and per fee

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Dan Williams
On Fri, Feb 10, 2017 at 12:17 PM, Greg Kroah-Hartman wrote: > On Fri, Feb 10, 2017 at 11:41:20AM -0800, Dan Williams wrote: >> On Fri, Feb 10, 2017 at 11:19 AM, Logan Gunthorpe >> wrote: >> > I copied this code and per feedback from Greg Kroah-Hartman [1] the >> > cdev's kobject's parent should

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Greg Kroah-Hartman
On Fri, Feb 10, 2017 at 11:41:20AM -0800, Dan Williams wrote: > On Fri, Feb 10, 2017 at 11:19 AM, Logan Gunthorpe wrote: > > I copied this code and per feedback from Greg Kroah-Hartman [1] the > > cdev's kobject's parent should not be set to the related device. > > This should have minor consequen

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Greg Kroah-Hartman
On Fri, Feb 10, 2017 at 12:19:30PM -0700, Logan Gunthorpe wrote: > I copied this code and per feedback from Greg Kroah-Hartman [1] the > cdev's kobject's parent should not be set to the related device. > This should have minor consequences but isn't doing what anyone > expects it to. > > This patc

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Dan Williams
On Fri, Feb 10, 2017 at 11:19 AM, Logan Gunthorpe wrote: > I copied this code and per feedback from Greg Kroah-Hartman [1] the > cdev's kobject's parent should not be set to the related device. > This should have minor consequences but isn't doing what anyone > expects it to. > > This patch then f

Re: [PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Logan Gunthorpe
Hey, Also on the subject of very minor fixes: I noticed drivers/dax is not in the maintainers file. I just assumed the nvdimm list should have been included with those from get_maintainers. Thanks, Logan On 10/02/17 12:19 PM, Logan Gunthorpe wrote: > I copied this code and per feedback from Gre

[PATCH] device-dax: don't set kobj parent during cdev init

2017-02-10 Thread Logan Gunthorpe
I copied this code and per feedback from Greg Kroah-Hartman [1] the cdev's kobject's parent should not be set to the related device. This should have minor consequences but isn't doing what anyone expects it to. This patch then fixes device-dax so it doesn't make the same mistake. [1] https://lkm