[dm-devel] dm-mirror: fix crash with mirror recovery and discard
Mike Snitzer
snitzer at redhat.com
Mon Jul 9 12:16:38 UTC 2012
On Fri, Jul 06 2012 at 4:38pm -0400,
Mikulas Patocka <mpatocka at redhat.com> wrote:
> > > + ti->discard_zeroes_data_unsupported = 1;
> > >
> > > ms->kmirrord_wq = alloc_workqueue("kmirrord",
> > > WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
> >
> > This should be split out to a separate patch and properly justified in
> > the patch header. Is there something unique to dm-mirror that renders
> > the underlying device's zeroing unreliable?
>
> There are two possible approaches to handling REQ_DISCARD
>
> 1. treat REQ_DISCARD as REQ_FLUSH (this is what the patch does) --- i.e.
> do not synchronize it with region states, do not set mirror error on
> failure. In this mode we must assume that there are uninitialized data
> after a flush.
>
> For example, if there is a region that is being resynchronized and we send
> REQ_DISCARD that overlaps this region, there is no guarantee that data in
> this region were zeroed.
>
> - kcopyd reads a few blocks for resynchronization
> - REQ_DISCARD is sent to both mirror legs, both disks overwrites the area
> with zeroes
> - kcopyd writes those blocks to the other leg => the blocks are no longer
> zero despite REQ_DISCARD being sent
OK, thanks, but my point still stands: this is worthy of a separate
patch (with the same type of backround you provided above).
> 2. treat REQ_DISCARD as writes (i.e. synchronize it with region states,
> wait until resynchronization finishes, etc.) --- it is possible to do it
> this way to, but if we do it this way, we have to split REQ_DISCARD on
> region boundaries (it is currently split only on target boundaries,
> which is insufficient).
I think discards should be treated as writes.
More information about the dm-devel
mailing list