Re: [PATCH V2 0/1] nvme: introduce generic per-namespace chardev

From: Minwoo Im <minwoo.im.dev@gmail.com>
To: Niklas Cassel <Niklas.Cassel@wdc.com>
Cc: "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"Keith Busch" <kbusch@kernel.org>, "Jens Axboe" <axboe@fb.com>,
	"Christoph Hellwig" <hch@lst.de>,
	"Sagi Grimberg" <sagi@grimberg.me>,
	"Kanchan Joshi" <joshiiitr@gmail.com>,
	"Javier González" <javier.gonz@samsung.com>
Subject: Re: [PATCH V2 0/1] nvme: introduce generic per-namespace chardev
Date: Tue, 6 Apr 2021 22:35:33 +0900	[thread overview]
Message-ID: <20210406133533.GC2594@localhost> (raw)
In-Reply-To: <YGwjc8rliXhvccPp@x1-carbon.lan>

On 21-04-06 09:01:41, Niklas Cassel wrote:
> On Tue, Apr 06, 2021 at 03:48:40PM +0900, Minwoo Im wrote:
> > Hello,
> > 
> > This is the second patch series to support generic ns character device
> > to expose per-namespace instance to the userspace.  This version fixed
> > code mis-ordered reported by Kanchan.
> > 
> > This patch introduces per-namespace character device to I/O in case that
> > blkdev is not initialized properly.  Userspace applications are able to
> > I/O to the generic namespace chardev even there's no blkdev properly
> > initialized.  Because we don't allow nvme controller device to I/O with
> > a specified nsid, this generic device will provide a way to I/O.
> > 
> > This patch is derived from Javier's patch series [1].  Javier and I have
> > re-coded this series again and it starts with new version tag.  Changes
> > from the previous series are:
> > 
> >   - Update naming convention for the chardev exactly the same with the
> >     blkdev:
> >         /dev/nvme-generic-XcYnZ  to  /dev/nvme-generic-XnY
> 
> Hello Minwoo,
> 
> The current proposal puts these new per-ns char devs in directly in
> /dev/ (I assume since Christoph didn't like the /dev/nvme/ subdir idea.
> Keith seemed to like the subdir idea, since he had suggested the same.)
> 
> For the absolute majority of cases, the namespace will not be rejected,
> so the user will be able to use the per-ns block dev to perform IOCTLs.
> 
> For the small minority of cases, Linux might reject the ns, so no block
> dev will be created.
> 
> I'm slightly worried that adding all these new per-ns char devs, in the
> same directory as the regular per-ns block devs, will lead to confusion
> from regular Linux users.
>
> Imagine the potential confusion about what device the user should use
> with e.g. fdisk, mkfs, mount, in fstab, what to specify in fstab, etc.
> 
> I think that there is value in reducing the confusion for regular users.

Agreed on this point.  We might have thousands of namespaces and it
might be making confusions to users.

> I don't know the best way of reducing this potential confusion, but here
> are some suggestions (suggestions are mutually exclusive):
> 
> 1) Put the new per-ns char devs in a directory different from where
> the regular per-ns block devs are located.
> 
> 2) Only create the new per-ns char dev for namespaces that were rejected.

I prefer this one which is the major reason of this patch series being
posted.

> 3) There is already a new module parameter for this, default it to false.
> 
> 4) Introduce a sysfs /sys/class/nvme/nvme0/export_unsupported_namespace
> that you can echo the nsid to, if you want to create the new per-ns char
> dev for a certain ns.
> 
> 
> I'm certain that someone can come up with an even better suggestion.

Thanks for your suggestion.  It sounds like it totally worth to be
discussed with people.  I'd also like to hear from other's opinions :)

Thanks!

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme