Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 4 Aug 2020 17:27:17 -0700 | Subject | Re: [GIT PULL] configfs updates for 5.9 |
| |
On Mon, Aug 3, 2020 at 7:07 AM Christoph Hellwig <hch@infradead.org> wrote: > > - propagate binary attribute errors (Lenny Szubowicz)
I pulled this, but I then unpulled again.
The commit is completely confused and wrong.
In particular, this part:
+ /* Only commit the data if no more shared refs to file */ + if (file_count(file) > 1) + return 0;
is bogus and prone to races, meaning that if there are multiple openers, *none* of them flush.
You can have two threads that close() at (roughly) the same time, both call ->flush(), and both of those see "file_count()" being 2 and never do anything.
They then both call "fput()" and are done, and that's when the file count gets decremented.
The fact is, "flush()" is called for each close(), and you should flush any pending data. Checking for some file_count() thing is bogus and completely wrong.
So make up your mind: either you flush synchronously at *every* close() (->flush) and you can get error notification.
Or you flush at the last close (->release), but then you absolutely cannot get any error reporting, because the last put of the file may be long after the last close (because other things than just open/close can increase the refcount).
You can't try to do some middle ground, because it _will_ be buggy. You have the above two options, not some racy third one.
Linus
| |