Re: [PATCH 2/2] fs: expose LOOKUP_NONBLOCK through openat2() RESOLVE_NONBLOCK

[Dave Chinner <david@fromorbit.com>]

On Thu, Dec 10, 2020 at 05:01:44PM -0800, Linus Torvalds wrote:
> On Thu, Dec 10, 2020 at 4:58 PM Dave Chinner <david@fromorbit.com> wrote:
> >
> > Umm, yes, that is _exactly_ what I just said. :/
> 
> .,. but it _sounded_ like you would actually want to do the whole
> filesystem thing, since why would you have piped up otherwise. I just
> wanted to clarify that the onle sane model is the one that patch
> actually implements.

<sigh>

Is that really what you think motivates me, Linus? It sounds like
you've created an Evil Dave strawman and you simply cannot see past
the taint Evil Dave has created in your head. :/

I commented because Jens has recently found several issues with
inconsistencies in "non-blocking" APIs that we have in the IO path.
He's triggered bugs in the non-blocking behaviour in filesystem code
through io_uring that we've had to fix (e.g. see commit 883a790a8440
("xfs: don't allow NOWAIT DIO across extent boundaries"). Then there
are the active discussions about the limited ability to use
IOCB_NOWAIT for full stack non-blocking IO behaviour w/ REQ_NOWAIT
in the block layer because the semantics of IOCB_NOWAIT are directly
tied to the requirements of the RWF_NOWAIT preadv2/pwritev2 flag and
using REQ_NOWAIT in the block layer will break them.

Part of the problem we have with the non-blocking behaviour is that
the user interfaces have been under specified, poorly reviewed and
targetted a single specific use case on a single filesystem rather
than generic behaviour. And mostly they lack the necessary test
coverage to ensure all filesystems behave the same way and to inform
us of a regression that *would break userspace applications*.

Yes, I recognise and accept that some of the problems are partially
my fault. I also have a habit of trying to learn from the mistakes
I've made and then take steps to ensure that *we do not make those
same mistakes again*.

> Otherwise, your email was just nit-picking about a single word in a
> comment in a header file.
> 
> Was that really what you wanted to do?

So for all your talk about "don't break userspace", you think that
actively taking steps during reviews to avoid a poor userspace API
is "nit-picking"? FYI, having a reviewer ask for a userspace API
modification to:

	- have clearly specified and documented behaviour,
	- be provided with user documentation, and
	- be submitted with regression tests

is not at all unusual or unexpected. Asking for these things during
review on -fsdevel and various filesystem lists is a normal part of
the process for getting changes to user APIs reviewed and merged.
The fact that Jens replied with "yep, no problems, let's make sure
we nail down the semantics" and Al has replied "what does
RESOLVE_NONBLOCK actually mean for all the blocking stuff that open
does /after/ the pathwalk?" shows that these semantics really do
matter Hence they need to be defined, specified, documented and
carefully exercised by regression tests. i.e. the patch that
introduces the RESOLVE_NONBLOCK flag is the -easy part-, filling in
the rest of the blanks is where all the hard work is...

Hence calling these requests "nit picking" sets entirely the wrong
tone for the wider community. You may not care about things like
properly documenting interfaces, but application developers and
users sure do and hence it's something we need to pay attention to
and encourage.

Leaders are supposed to encourage and support good development
practices, not be arseholes to the people who ask for good practices
to be followed.

Please start behaving more like a leader should when I'm around,
Linus.

-Dave.
(Not really Evil.)
-- 
Dave Chinner
david@fromorbit.com