On Sun, 25 Feb 2024 at 17:03, Kent Overstreet <kent.overstreet@xxxxxxxxx> wrote:
>
> We could satisfy the posix atomic writes rule by just having a properly
> vectorized buffered write path, no need for the inode lock - it really
> should just be extending writes that have to hit the inode lock, same as
> O_DIRECT.
>
> (whenever people bring up range locks, I keep trying to tell them - we
> already have that in the form of the folio lock, if you'd just use it
> properly...)
Sadly, that is *technically* not proper.
IOW, I actually agree with you that the folio lock is sufficient, and
several filesystems do too.
BUT.
Technically, the POSIX requirements are that the atomicity of writes
are "all or nothing" being visible, and so ext4, for example, will
have the whole write operation inside the inode_lock.
Note that this is not some kind of locking requirement coming from
some ext4 locking rule: it only does this for the buffered path, the
DIO path happily says "if I'm not extending the size of the inode,
I'll just take the lock for shared access".
So this is literally only about buffered writes, and the atomicity
within a folio is already dealt with with the folio lock (ext4 uses
"generic_perform_write()" to do the actual writing part, which does
*not* care about that lock).
IOW, the whole inode lock is pointless for any real uses, but exists
because _technically_ POSIX does not allow reads to see partial writes
(ie if you see *one* page change, you have to see all of them change,
so per-folio locking is not sufficient - you technically need to lock
the whole operations against readers too, not just writers).
Of course, in real life absolutely nobody cares, and you can see
partial writes in many other ways, so this is really a completely
"because of paper standards" serialization.
Several other filesystems do *not* do that serialization, and as
mentioned, the DIO paths also just said "hey, this isn't a normal
write, so we'll ignore POSIX because technically we can".
But to take a different example, ext2 just calls
generic_file_write_iter() *without* taking the inode lock, and does
locking one page at a time. As far as I know, nobody ever really
complained.
(It's not just ext2. It's all the old filesystems: anything that uses
generic_file_write_iter() without doing inode_lock/unlock around it,
which is actually most of them).
And I find that silly locking rule a bit offensive, because it
literally serializes accesses that shouldn't be serialized.
IOW, it *should* be a "you do stupid things, you get what you
deserve". Instead, it's a "everybody pays the price for giving stupid
things a pointless semantic guarantee".
Afaik, it has a purely historical reason for it - all the original
UNIX read/write calls used a per-inode lock, so they were all
serialized whether you liked it or not.
Oh well. It's a pet peeve of mine.
Linus
