Re: [RFC] Re: broken userland ABI in configfs binary attributes

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, Aug 27, 2019 at 06:01:27PM +0300, Boaz Harrosh wrote:
> On 26/08/2019 22:32, Al Viro wrote:
> <>
> > D'oh...  OK, that settles it; exclusion with st_write() would've been
> > painful, but playing with the next st_write() on the same struct file
> > rewinding the damn thing to overwrite what st_flush() had spewed is
> > an obvious no-go.
> > 
> 
> So what are the kind of errors current ->release implementation is trying to return?
> Is it actual access the HW errors or its more of a resource allocations errors?
> If the later then maybe the allocations can be done before hand, say at ->flush but
> are not redone on redundant flushes?

Most of them are actually pure bollocks - "it can never happen, but if it does,
let's return -EWHATEVER to feel better".  Some are crap like -EINTR, which is
also bollocks - for one thing, process might've been closing files precisely
because it's been hit by SIGKILL.  For another, it's a destructor.  It won't
be retried by the caller - there's nothing called for that object afterwards.
What you don't do in it won't be done at all.

And some are "commit on final close" kind of thing, both with the hardware
errors and parsing errors.

> If the former then yes looks like troubles. That said I believe the 
> ->last_close_with_error() is a very common needed pattern which few use exactly
> because it does not work. But which I wanted/needed many times before.
> 
> So I would break some eggs which ever is the most elegant way, and perhaps add a
> new parameter to ->flush(bool last) or some other easy API.
> [Which is BTW the worst name ever, while at it lets rename it to ->close() which
>  is what it is. "flush" is used elsewhere to mean sync.
> ]

It *is* flush.  And nothing else, really - it makes sure that dirty data is
pushed to destination, with any errors reported.

> So yes please lets fix VFS API with drivers so to have an easy and safe way
> to execute very last close, all the while still being able to report errors to
> close(2).

What the hell is "very last close()"?  Note, BTW, that you can have one thread
call close() in the middle of write() by another thread.  And that will succeed,
with actual ->release() happening no earlier than write() is done; however,
descriptor will be gone when close(2) is done.

You are assuming the model that doesn't match the basic userland ABI for
descriptors.  And no, close(2) is not going to wait for write(2) to finish.
Neither it is going to interrupt said write(2).

Note, BTW, that an error returned by close(2) does *NOT* leave the descriptor
in place - success or failure, it's gone.

If you want to express something like "data packet formed; now you can commit
it and tell me if there'd been any errors", use something explicit.  close()
simply isn't suitable for that.  writev() for datagram-like semantics might
be; fsync() or fdatasync() could serve for "commit now".