Re: [PATCH] fs: ratelimit __find_get_block_slow() failure message.

From: Jan Kara <jack@suse.cz>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Jan Kara <jack@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH] fs: ratelimit __find_get_block_slow() failure message.
Date: Wed, 16 Jan 2019 11:43:08 +0100	[thread overview]
Message-ID: <20190116104308.GC26069@quack2.suse.cz> (raw)
In-Reply-To: <CACT4Y+ZyP2QGL3ENJZRe8qbiL8SB8yDPHRY-npTzb14+LB05RQ@mail.gmail.com>

On Wed 16-01-19 10:47:56, Dmitry Vyukov wrote:
> On Fri, Jan 11, 2019 at 1:46 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >
> > On 2019/01/11 19:48, Dmitry Vyukov wrote:
> > >> How did you arrive to the conclusion that it is harmless?
> > >> There is only one relevant standard covering this, which is the C
> > >> language standard, and it is very clear on this -- this has Undefined
> > >> Behavior, that is the same as, for example, reading/writing random
> > >> pointers.
> > >>
> > >> Check out this on how any race that you might think is benign can be
> > >> badly miscompiled and lead to arbitrary program behavior:
> > >> https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong
> > >
> > > Also there is no other practical definition of data race for automatic
> > > data race detectors than: two conflicting non-atomic concurrent
> > > accesses. Which this code is. Which means that if we continue writing
> > > such code we are not getting data race detection and don't detect
> > > thousands of races in kernel code that one may consider more harmful
> > > than this one the easy way. And instead will spent large amounts of
> > > time to fix some of then the hard way, and leave the rest as just too
> > > hard to debug so let the kernel continue crashing from time to time (I
> > > believe a portion of currently open syzbot bugs that developers just
> > > left as "I don't see how this can happen" are due to such races).
> > >
> >
> > I still cannot catch. Read/write of sizeof(long) bytes at naturally
> > aligned address is atomic, isn't it?
> 
> Nobody guarantees this. According to C non-atomic conflicting
> reads/writes of sizeof(long) cause undefined behavior of the whole
> program.

Yes, but to be fair the kernel has always relied on long accesses to be
atomic pretty heavily so that it is now de-facto standard for the kernel
AFAICT. I understand this makes life for static checkers hard but such is
reality.

But in this particular case I agree with you that special logic is not
really warranted.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR