[ adding Neil, linux-raid ]
On Tue, Oct 13, 2015 at 5:26 AM, hch@infradead.org [off-list ref] wrote:
On Tue, Oct 13, 2015 at 01:53:34AM +0000, Williams, Dan J wrote:
quoted
...i.e. that we're destroying the integrity profile while i/o is still
in flight. As far as I can see any driver that calls
blk_integrity_unregister() before blk_cleanup_queue() can hit this.
However, with the change to static allocation I'm not sure why a driver
would ever need to call blk_integrity_unregister() in its shutdown path.
It shouldn't.
quoted
It seems this would only be necessary for disabling integrity at run
time, but it can only do it safely when the queue is known to be idle.
Yes. And even for that case we should a) only clear ->flags not the
whole integrity profile (and fix blk_integrity_revalidate to check the
right thing) and b) clear flags before calling blk_integrity_revalidate.
quoted
Is there a way to solve this without the generic blk_freeze_queue()
implementation? [1]. The immediate fix for libnvdimm is to just stop
calling blk_integrity_unregister().
Seems like only nvme ever updates the profile, and nvme is blk-mq
only.
Looks like both dm and md are calling blk_integrity_unregister() at
run-time as well when adding new disks to the configuration. Even if
we fix blk_integrity_unregister() to not trigger a crash it seems we
still need to stop all I/O during the unregistration so we don't throw
spurious integrity miscompares for in-flight i/o.
For dm I'm wondering what's the difference between a mapped_device
being suspended vs having completed a stop_queue()? Mike?
For md looks like a simple mddev_suspend()/resume() in
md_integrity_add_rdev() is sufficient.
...and then blk_mq_freeze_queue() for nvme in nvme_revalidate_disk().
I'll throw together a patch...