Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Nicholas Piggin <npiggin@gmail.com>,
	Dave Chinner <david@fromorbit.com>,
	akpm@linux-foundation.org, Marco Elver <elver@google.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, "Paul E. McKenney" <paulmck@kernel.org>,
	syzbot <syzbot+0f1e470df6a4316e0a11@syzkaller.appspotmail.com>,
	syzkaller-bugs@googlegroups.com, Will Deacon <will@kernel.org>
Subject: Re: KCSAN: data-race in generic_file_buffered_read / generic_file_buffered_read
Date: Thu, 16 Jul 2020 14:19:29 +0100	[thread overview]
Message-ID: <20200716131929.GL12769@casper.infradead.org> (raw)
In-Reply-To: <20200716065454.GI1167@sol.localdomain>

On Wed, Jul 15, 2020 at 11:54:54PM -0700, Eric Biggers wrote:
> > >> > Concurrent reads on the same file descriptor are allowed.  Not with sys_read(),
> > >> > as that implicitly uses the file position.  But it's allowed with sys_pread(),
> > >> > and also with sys_sendfile() which is the case syzbot is reporting here.
> > >> 
> > >> Concurrent read()s are fine, they'll just read from the same offset.
> > >> 
> > > 
> > > Actually the VFS serializes concurrent read()'s on the same fd, at least for
> > > regular files.
> > 
> > Hmm, where?
> 
> It's serialized by file->f_pos_lock.  See fdget_pos().

What if we trylock either f_lock or f_pos_lock for readahead and just
skip all the readahead code if it's already taken?  I'd suggest that if
there are two readers using the same struct file, this is probably not
a workload that benefits greatly from readahead.


      parent reply	other threads:[~2020-07-16 13:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <0000000000004a4d6505aa7c688a@google.com>
     [not found] ` <20200715152912.GA2209203@elver.google.com>
2020-07-15 16:32   ` Eric Biggers
2020-07-15 16:45     ` Marco Elver
2020-07-15 23:42     ` Dave Chinner
2020-07-16  3:03       ` Eric Biggers
2020-07-16  6:24         ` Nicholas Piggin
2020-07-16  6:54           ` Eric Biggers
2020-07-16  7:52             ` Nicholas Piggin
2020-07-16 12:56               ` Marco Elver
2020-07-16 13:19             ` Matthew Wilcox [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=20200716131929.GL12769@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=ebiggers@kernel.org \
    --cc=elver@google.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=syzbot+0f1e470df6a4316e0a11@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=will@kernel.org \
    --subject='Re: KCSAN: data-race in generic_file_buffered_read / generic_file_buffered_read' \
    /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).