Block integrity registration update

Dan Williams dan.j.williams at intel.com
Tue Oct 13 10:38:13 PDT 2015


[ adding Neil, linux-raid ]

On Tue, Oct 13, 2015 at 5:26 AM, hch at infradead.org <hch at infradead.org> wrote:
> On Tue, Oct 13, 2015 at 01:53:34AM +0000, Williams, Dan J wrote:
>> ...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.
>
>> 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.
>
>> 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...



More information about the Linux-nvme mailing list