Re: [PATCH v3 2/2] nvme: fix cdev lifetime

[John Garry <john.g.garry@oracle.com>]

On 16/07/2026 16:03, Nilay Shroff wrote:
> On 7/13/26 4: 12 PM, John Garry wrote: > Fix the lifetime for the cdev by making
> adding the cdev add take a > reference to the NS head and drop that reference in
> the > nvme_ns_head. cdev_device release function. > > The same problem
> 
> 
> On 7/13/26 4:12 PM, John Garry wrote:
>> Fix the lifetime for the cdev by making adding the cdev add take a
>> reference to the NS head and drop that reference in the
>> nvme_ns_head.cdev_device release function.
>> 
>> The same problem exists for the NS cdev lifetime, so resolve that issue
>> through a similar method by taking a reference to the NS for the lifetime
>> of the cdev. Note that nvme_ns_chr_open() -> nvme_ns_open() also takes a
>> reference to the NS. Now that should not be needed, but that code is
>> common to bdev ioctl, so keep as is.
> 
> The bdev ioctl uses nvme_ns_open() and nvme_ns_release(). So, IMO, you may also
> remove nvme_ns_chr_open() and nvme_ns_chr_release() methods.

nvme_ns_chr_open() and nvme_ns_chr_release() call nvme_ns_open() and 
nvme_ns_relase(), respectively, and they do more than get and put a ref 
to the NS - specifically they also check that they are not called for 
multipath mode (as the NS bdev/cdev should be hidden) and take/put a 
reference to the controller ops module. Why are those addition actions 
not required for the cdev? Or should it be done when we add/del the cdev 
(like in this patch)?

> 
> Otherwise this looks good.
> 
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> 

thanks