Re: [patch 2/4] fs/buffer: Move BH_Uptodate_Lock locking into wrapper functions

On Wed, 31 Jul 2019, Jan Kara wrote:

On Tue 30-07-19 13:24:54, Thomas Gleixner wrote:

quoted

Bit spinlocks are problematic if PREEMPT_RT is enabled, because they
disable preemption, which is undesired for latency reasons and breaks when
regular spinlocks are taken within the bit_spinlock locked region because
regular spinlocks are converted to 'sleeping spinlocks' on RT. So RT
replaces the bit spinlocks with regular spinlocks to avoid this problem.

To avoid ifdeffery at the source level, wrap all BH_Uptodate_Lock bitlock
operations with inline functions, so the spinlock substitution can be done
at one place.

Using regular spinlocks can also be enabled for lock debugging purposes so
the lock operations become visible to lockdep.

Signed-off-by: Thomas Gleixner <redacted>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

BTW, it should be possible to get rid of BH_Uptodate_Lock altogether using
bio chaining (which was non-existent when this bh code was written) to make
sure IO completion function gets called only once all bios used to fill in
/ write out the page are done. It would be also more efficient. But I guess
that's an interesting cleanup project for some other time...

While 'possible cleanup' is something which triggers a certain nerve, that
particular project certainly goes beyond my basic understanding of that
whole fs/block conglomerate. I rather leave that to people who actually
have a clue. :)

Thanks,

	tglx