linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Subject: Re: [md PATCH 4/4] md/bitmap: Don't write bitmap while earlier writes might be in-flight
Date: Mon, 07 Nov 2016 09:53:42 +1100	[thread overview]
Message-ID: <87r36onrt5.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <20161105003325.qnmcu363ocbli74x@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

On Sat, Nov 05 2016, Shaohua Li wrote:

> On Fri, Nov 04, 2016 at 04:46:03PM +1100, Neil Brown wrote:
>> As we don't wait for writes to complete in bitmap_daemon_work, they
>> could still be in-flight when bitmap_unplug writes again.  Or when
>> bitmap_daemon_work tries to write again.
>> This can be confusing and could risk the wrong data being written last.
>
> Applied the first 3 patches, thanks!
>
> This one seems not completely solving the race condition. It's still possible
> bitmap_daemon_work clears BITMAP_PAGE_NEEDWRITE but hasn't dispatch the IO yet,
> bitmap_unplug then does nothing and thinks bitmap is updated to disk. Why don't
> we add locking here?

Thanks for the review!

BITMAP_PAGE_NEEDWRITE is set for pages that need to be written out in
order to clear bits that don't need to be set any more.  There is never
any urgency to do this.
BITMAP_PAGE_DIRTY is set of pages that need to be written out in order
to set bits representing regions that are about to be written to.  These
have to be flushed by bitmap_unplug().
Pages can have both bits set, in which case bitmap_daemon_work() will
leave them for bitmap_unplug() to deal with.

So if bitmap_daemon_work() clears BITMAP_PAGE_PENDING on a page, then it
is a page that bitmap_unplug() doesn't need to wait for.


Does that answer your concerns?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

  reply	other threads:[~2016-11-06 22:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-04  5:46 [md PATCH 0/4] Assorted minor improvements NeilBrown
2016-11-04  5:46 ` [md PATCH 4/4] md/bitmap: Don't write bitmap while earlier writes might be in-flight NeilBrown
2016-11-05  0:33   ` Shaohua Li
2016-11-06 22:53     ` NeilBrown [this message]
2016-11-07 19:19       ` Shaohua Li
2016-11-07 20:19         ` NeilBrown
2016-11-07 22:57           ` Shaohua Li
2016-11-04  5:46 ` [md PATCH 1/4] md: perform async updates for metadata where possible NeilBrown
2016-11-04  5:46 ` [md PATCH 3/4] md/raid10: abort delayed writes when device fails NeilBrown
2016-11-04  5:46 ` [md PATCH 2/4] md/raid1: " NeilBrown

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=87r36onrt5.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=shli@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).