Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 16 Jul 2020 14:19:29 +0100
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
Message-ID: <20200716131929.GL12769@casper.infradead.org>
References: <0000000000004a4d6505aa7c688a@google.com>
 <20200715152912.GA2209203@elver.google.com>
 <20200715163256.GB1167@sol.localdomain>
 <20200715234203.GK5369@dread.disaster.area>
 <20200716030357.GE1167@sol.localdomain>
 <1594880070.49b50i0a1p.astroid@bobo.none>
 <20200716065454.GI1167@sol.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200716065454.GI1167@sol.localdomain>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.