Re: [PATCH 2/2] bcachefs: Buffered write path now can avoid the inode lock

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Kent Overstreet <kent.overstreet@linux.dev>
Cc: linux-bcachefs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	 david@fromorbit.com, mcgrof@kernel.org, hch@lst.de,
	willy@infradead.org
Subject: Re: [PATCH 2/2] bcachefs: Buffered write path now can avoid the inode lock
Date: Wed, 28 Feb 2024 23:27:05 -0800	[thread overview]
Message-ID: <CAHk-=wg96Rt-SuUeRb-xev1KdwqX0GLFjf2=qnRsyLimx6-xzw@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=whf9HsM6BP3L4EYONCjGawAV=X0aBDoUHXkND4fpqB2Ww@mail.gmail.com>

On Wed, 28 Feb 2024 at 23:20, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
>  - take the lock exclusively if O_APPEND or if it *looks* like you
> might extend the file size.
>
>  - otherwise, take the shared lock, and THEN RE-CHECK. The file size
> might have changed, so now you need to double-check that you're really
> not going to extend the size of the file, and if you are, you need to
> go back and take the inode lock exclusively after all.

Same goes for the suid/sgid checks. You need to take the inode lock
shared to even have the i_mode be stable, and at that point you might
decide "oh, I need to clear suid/sgid after all" and have to go drop
the lock and retake it for exclusive access after all.

(And yes, we *really* should have a rwsem_try_upgrade() operation and
at least avoid the "read_unlock -> write_lock" dance), but we don't.
See a comment in mmap_upgrade_trylock() about it. It should be
reasonably easy to add, but we've never really had enough reason to.
Maybe somebody decides it's worth the effort)

               Linus