Re: [PATCH 07/10] netfs: Fix missing barriers by using clear_and_wake_up_bit()

Akira Yokosawa
2024-12-14T10:16:27Z

Hi David,

David Howells wrote:
> Use clear_and_wake_up_bit() rather than something like:
>
> clear_bit_unlock(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
> wake_up_bit(&rreq->flags, NETFS_RREQ_IN_PROGRESS);
>
> as there needs to be a barrier inserted between which is present in
> clear_and_wake_up_bit().

If I am reading the kernel-doc comment of clear_bit_unlock() [1, 2]:

This operation is atomic and provides release barrier semantics.

correctly, there already seems to be a barrier which should be
good enough.

[1]: https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.clear_bit_unlock
[2]: include/asm-generic/bitops/instrumented-lock.h

>
> Fixes: 288ace2f57c9 ("netfs: New writeback implementation")
> Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")

So I'm not sure this fixes anything.

What am I missing?

Thanks, Akira

> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Zilin Guan <zilin@seu.edu.cn>
> cc: Akira Yokosawa <akiyks@gmail.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/netfs/read_collect.c | 3 +--
> fs/netfs/write_collect.c | 9 +++------
> 2 files changed, 4 insertions(+), 8 deletions(-)
> …