mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Hugh Dickins <hughd@google.com>
Cc: Dave Chinner <david@fromorbit.com>,
	Jakob Unterwurzacher <jakobunt@gmail.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: tmpfs returns incorrect data on concurrent pread() and truncate()
Date: Wed, 2 Nov 2016 09:21:14 -0400	[thread overview]
Message-ID: <20161102132114.amvwgjxq7x2b7ns5@thunk.org> (raw)
In-Reply-To: <alpine.LSU.2.11.1611011833130.2382@eggly.anvils>

On Tue, Nov 01, 2016 at 06:38:26PM -0700, Hugh Dickins wrote:
> > Put simple: page locks are insufficient as a generic mechanism for
> > serialising filesystem operations. The locking required for this is
> > generally deeply filesystem implementation specific, so it's fine
> > that the VFS doesn't attempt to provide anything stricter than it
> > currently does....
> 
> I think you are saying that: xfs already provides the extra locking
> that avoids this issue; most other filesystems do not; but more can
> be expected to add that extra locking in the coming months?

The test program was using buffered reads, and ext4 is using
generic_file_read_iter(), so presumably adding file system specific
locking would require that file systems which use
generic_file_read_iter() would need to front-end it with the
additional required file system specific locks.  This is what XFS is
currently doing, but a quick git grep seems to show that besides XFS,
nfs, cifs, ceph and ocfs2 (i.e., XFS and remote file systems),
everyone else is using generic_file_read_iter() is using it directly
as read_iter method.

So if we think we really care about getting this case right, and if we
think the locking overhead in the uncontended case is sufficiently low
that it's worth fixing (it'll probably mean the bonnie benchmark will
take a hit, but that's a pretty crappy benchmark anyway :-), perhaps
we should be thinking about some way of adding a generic locking
mechanism so that most file systems will have a fighting chance of
getting this right.

Otherwise, perhaps the major file systems will add the appropriate
locking to the buffered I/O case (much more attention has been paid to
the direct I/O case, which is where more workloads have historically
cared) but I suspect as a percentage of the file systems in the Linux
kernel, they won't be getting this case right.

Cheers,

						- Ted

  parent reply	other threads:[~2016-11-02 13:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-26 17:05 Jakob Unterwurzacher
2016-11-01 23:51 ` Hugh Dickins
2016-11-02  1:01   ` Dave Chinner
2016-11-02  1:38     ` Hugh Dickins
2016-11-02  4:01       ` Dave Chinner
2016-11-02 13:21       ` Theodore Ts'o [this message]
2016-11-03 23:45   ` Jakob Unterwurzacher

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=20161102132114.amvwgjxq7x2b7ns5@thunk.org \
    --to=tytso@mit.edu \
    --cc=david@fromorbit.com \
    --cc=hughd@google.com \
    --cc=jakobunt@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.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

all inboxes | Powered by JetHome®