Re: [PATCH V2 2/2] nvmet: add per ns thread to generate AEN

From: Anthony Iliopoulos <ailiop@suse.com>
To: Sagi Grimberg <sagi@grimberg.me>
Cc: linux-nvme@lists.infradead.org,
	Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>,
	hch@lst.de
Subject: Re: [PATCH V2 2/2] nvmet: add per ns thread to generate AEN
Date: Wed, 8 Apr 2020 11:55:00 +0200	[thread overview]
Message-ID: <20200408095500.GD1329@technoir> (raw)
In-Reply-To: <5e54afaa-7de5-a89e-5740-88df15c52bba@grimberg.me>

On Tue, Apr 07, 2020 at 10:33:07PM -0700, Sagi Grimberg wrote:
> 
> 
> On 4/7/20 9:16 PM, Chaitanya Kulkarni wrote:
> > The change of size detection on the target should generate the AEN to
> > the host. Right now there is no mechanism that allows us to add
> > callbacks for the block and file backend so that we will get the
> > notification for change of the size for block device and file backend.
> > This patch adds a simple per namespace thread that checks for the size
> > change and generates AEN when needed.
> 
> kthread? per ns?! I really don't think this is the way to go Chaitanya..
> 
> I'd suggest to expose a revalidate configfs attribute and have nvmetcli
> install a udev rule that triggers a write to this attribute. Balbir
> already got the udev notification for disk resize (see
> set_capacity_revalidate_and_notify).

So in the original design I had assumed that the burden of AEN
generation falls on userspace, given that the resizing of either a block
device or a file is something that userspace would initiate anyway. As
such, the same process could simply issue a nvme rescan ioctl to effect
the capacity changes (this is why in the original patch I had wired up
revalidate with identify).

With the udev notification patchset in place one could trigger the
rescan for bdev-backed targets (e.g. do a nvme ns-rescan, or toggle
the existing nvmet ns enable configfs attr to induce the change). Some
revalidation actions will necessarily remain in userspace, e.g. in
blktests where loopback bdev is used, it requires a LOOP_SET_CAPACITY
ioctl to reflect the changes (losetup -c).

For file-backed targets, userspace may leverage inotify to achieve the
same. Otherwise a size change notifier would be required to trigger a
rescan (e.g. see fs/attr.c:notify_change()). The issue though here is
that there is no immediate way to associate a filp to a namespace, which
means that the handler would need to iterate all controllers and every
namespace to match it - not very nice, and probably isn't worth the
complexity.

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