[ adding Neil, linux-raid ] On Tue, Oct 13, 2015 at 5:26 AM, hch@xxxxxxxxxxxxx <hch@xxxxxxxxxxxxx> 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... -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
