All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Whitney <enwlinux@gmail.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Jan Kara <jack@suse.cz>, Eric Whitney <enwlinux@gmail.com>,
	linux-ext4@vger.kernel.org
Subject: Re: [PATCH] ext4: revert i_data_sum locking cleanups for dioread_nolock
Date: Fri, 19 Feb 2016 09:19:25 -0500	[thread overview]
Message-ID: <20160219141925.GA20062@localhost.localdomain> (raw)
In-Reply-To: <20160219053047.GD12743@thunk.org>

* Theodore Ts'o <tytso@mit.edu>:
> On Thu, Feb 18, 2016 at 11:09:56PM +0100, Jan Kara wrote:
> > OK, I had a look into this. So I'm not 100% what has happened but the
> > following looks likely: Current io_end handling can overwrite io_end
> > pointer in the inode in dioread_nolock mode (nothing prevents unlocked DIO
> > to overwrite pointer of locked DIO and then clear it out). I suspect that
> > the change in i_data_sem locking made this race more visible. Attached
> > patch should fix the issue (I don't see failures of generic/300 with it in
> > dioread_nolock mode). Can you consider this instead of a revert Eric sent?
> 
> Thanks!  That does appear to be it. I dropped the revert, confirmed
> that I could still trivially reproduce the failure, applied patch,
> and ran the test 10 times ("kvm-xfstests -C 10 -c dioread_nolock
> generic/300") and it passed with flying colors.
> 
> > I have also a more complete rewrite of io_end handling which makes the code
> > more comprehensible and avoids storing io_end pointer in the inode (thus
> > avoids similar pitfalls in future) but that is a 4.6 matter. I'll submit
> > the rewrite once xfstests runs complete.
> 
> Great, thanks!
> 
> 					- Ted

I ran the same ten test runs (kvm-xfstests -c dioread_nolock generic/300) on
x86_64 and a full test run (kvm-xfstests -g auto) with the patch applied to
4.5-rc4 without regressions relative to my -rc4 baseline results.  Looks
good to me.

Tested-by: Eric Whitney <enwlinux@gmail.com>



      reply	other threads:[~2016-02-19 14:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-12 18:25 [PATCH] ext4: revert i_data_sum locking cleanups for dioread_nolock Eric Whitney
2016-02-16  5:08 ` Theodore Ts'o
2016-02-18 22:09   ` Jan Kara
2016-02-19  5:30     ` Theodore Ts'o
2016-02-19 14:19       ` Eric Whitney [this message]

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=20160219141925.GA20062@localhost.localdomain \
    --to=enwlinux@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.