Re: [GIT PULL] configfs updates for 5.9

[Christoph Hellwig]

On Tue, Aug 04, 2020 at 05:59:19PM -0700, Linus Torvalds wrote:
> So again, doing that
> 
>         if (file_count(file) > 1)
> 
> kind of check is very very wrong even outside of the fundamental race
> with two close() calls at the same time.
> 
> It is a very dangerous pattern, because it likely works in practice
> during testing, and looks like it might work.
> 
> But it is completely and unfixably wrong.
> 
> Again, the only reliable way to do that "last close" is "->release()",
> but you will never get errors from that, since (for all the same
> reasons) it might not even be done by a close. The last releaser of a
> file descriptor might be that mmap/io_uring/proc case now releasing
> the no longer used file, possibly long after the last "close()" call
> has happened.
> 
> One acceptable half-way measure *may* be
> 
>  - do the flush with the above bogus test at ->flush() time, knowing
> it might never trigger at all
> 
>  - do the flush *again* at ->release() time, in case it didn't trigger
> 
>  - add a big comment to the flush-time case to show you understand the
> issue, and understand
> 
> but I'd discourage it because of how unreliable it is.

Yes, but this is a fundamental problem with the commit on close
model that the configfs binary attributes implement, and we have to
work around that somehow.  The current behavior is to entirely ignore
errors, which of course has major issues.  I guess the alternative might
be to just commit on every ->flush as the use case (and yes I know
people could abuse it) for the configfs files is a set of simple
write syscalls.  Then again the above might be better than that.

Lenny, can you look ingo that?