LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Wheeler <drbd-dev@lists.ewheeler.net>
To: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	drbd-dev@lists.linbit.com, stable@vger.kernel.org,
	Philipp Reisner <philipp.reisner@linbit.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drbd: fix discard_zeroes_if_aligned regression
Date: Fri, 10 May 2019 17:36:32 +0000 (UTC)	[thread overview]
Message-ID: <alpine.LRH.2.11.1905101728280.1835@mx.ewheeler.net> (raw)
In-Reply-To: <20180116094907.GD4107@soda.linbit>

On Tue, 16 Jan 2018, Lars Ellenberg wrote:

> On Mon, Jan 15, 2018 at 11:26:15PM -0800, Christoph Hellwig wrote:
> > NAK.  Calling a discard and expecting zeroing is simply buggy.
> 
> What he said.
> 
> The bug/misunderstanding was that we now use zeroout even for discards,
> with the assumption that it would try to do discards where it can.
> 
> Which is even true.
> 
> But our expectations of when zeroout "should" use unmap,
> and where it actually can do that safely
> based on the information it has,
> don't really match:
> our expectations where wrong, we assumed too much.
> (in the general case).
> 
> Which means in DRBD we have to stop use zeroout for discards,
> and again pass down normal discard for discards.
> 
> In the specific case where the backend to DRBD is lvm-thin,
> AND it does de-alloc blocks on discard,
> AND it does NOT have skip_block_zeroing set or it's backend
> does zero on discard and it does discard passdown, and we tell
> DRBD about all of that (using the discard_zeroes_if_aligned flag)
> then we can do this "zero head and tail, discard full blocks",
> and expect them to be zero.
> 
> If skip_block_zeroing is set however, and there is no discard
> passdown in thin, or the backend of thin does not zero on discard,
> DRBD can still pass down discards to thin, and expect them do be
> de-allocated, but can not expect discarded ranges to remain
> "zero", any later partial write to an unallocated area could pull
> in different "undefined" garbage on different replicas for the
> not-written-to part of a new allocated block.
> 
> The end result is that you have areas of the block device
> that return different data depending on which replica you
> read from.
> 
> But that is the case even eithout discard in that setup,
> don't do that then or live with it.
> 
> "undefined data" is undefined, you have that directly on thin
> in that setup already, with DRBD on top you now have several
> versions of "undefined".
> 
> Anything on top of such a setup must not do "read-modify-write"
> of "undefined" data and expect a defined result, adding DRBD
> on top does not change that.
> 
> DRBD can deal with that just fine, but our "online verify" will
> report loads of boring "mismatches" for those areas.
> 
> TL;DR: we'll stop doing "discard-is-zeroout"
> (our assumptions were wrong).
> We still won't do exactly "discard-is-discard", but re-enable our
> "discard-is-discard plus zeroout on head and tail", because in
> some relevant setups, this gives us the best result, and avoids
> the false positives in our online-verify.
> 
>     Lars
> 

Hi Lars,

We just tried 4.19.x and this bugs still exists. We applied the patch 
which was originally submitted to this thread and it still applies cleanly 
and seems to work for our use case. You mentioned that you had some older 
code which zeroed out unaligned discard requests (or perhaps it was for a 
different purpose) that you may be able to use to patch this. Could you 
dig those up and see if we can get this solved?

It would be nice to be able to use drbd with thin backing volumes from the 
vanilla kernel. If this has already been fixed in something newer than 
4.19, then please point me to the commit.

Thank you for your help!

 
--
Eric Wheeler



  reply	other threads:[~2019-05-10 17:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <15124635.GA4107@soda.linbit>
2018-01-15 23:00 ` Eric Wheeler
2018-01-16  7:26   ` Christoph Hellwig
2018-01-16  9:49     ` Lars Ellenberg
2019-05-10 17:36       ` Eric Wheeler [this message]
2019-05-28 13:18         ` Lars Ellenberg
2019-06-02  0:28           ` Eric Wheeler
2018-01-17  0:43     ` Eric Wheeler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LRH.2.11.1905101728280.1835@mx.ewheeler.net \
    --to=drbd-dev@lists.ewheeler.net \
    --cc=drbd-dev@lists.linbit.com \
    --cc=hch@infradead.org \
    --cc=lars.ellenberg@linbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=philipp.reisner@linbit.com \
    --cc=stable@vger.kernel.org \
    --subject='Re: [PATCH] drbd: fix discard_zeroes_if_aligned regression' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).