From: Akira Yokosawa <akiyks@gmail.com>
To: David Howells <dhowells@redhat.com>,
Christian Brauner <christian@brauner.io>
Cc: Max Kellermann <max.kellermann@ionos.com>,
Ilya Dryomov <idryomov@gmail.com>, Xiubo Li <xiubli@redhat.com>,
Trond Myklebust <trondmy@kernel.org>,
Jeff Layton <jlayton@kernel.org>,
Matthew Wilcox <willy@infradead.org>,
netfs@lists.linux.dev, linux-afs@lists.infradead.org,
linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org,
ceph-devel@vger.kernel.org, v9fs@lists.linux.dev,
linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Zilin Guan <zilin@seu.edu.cn>, Akira Yokosawa <akiyks@gmail.com>
Subject: Re: [PATCH 07/10] netfs: Fix missing barriers by using clear_and_wake_up_bit()
Date: Sat, 14 Dec 2024 19:16:18 +0900 [thread overview]
Message-ID: <27fff669-bec4-4255-ba2f-4b154b474d97@gmail.com> (raw)
In-Reply-To: <20241213135013.2964079-8-dhowells@redhat.com>
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(-)
>
[...]
WARNING: multiple messages have this Message-ID (diff)
From: Akira Yokosawa <akiyks@gmail.com>
To: David Howells <dhowells@redhat.com>,
Christian Brauner <christian@brauner.io>
Cc: linux-cifs@vger.kernel.org,
Max Kellermann <max.kellermann@ionos.com>,
v9fs@lists.linux.dev, Akira Yokosawa <akiyks@gmail.com>,
Jeff Layton <jlayton@kernel.org>,
linux-nfs@vger.kernel.org, Matthew Wilcox <willy@infradead.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
ceph-devel@vger.kernel.org, Zilin Guan <zilin@seu.edu.cn>,
linux-fsdevel@vger.kernel.org, netfs@lists.linux.dev,
Ilya Dryomov <idryomov@gmail.com>, Xiubo Li <xiubli@redhat.com>,
linux-erofs@lists.ozlabs.org, linux-afs@lists.infradead.org,
Trond Myklebust <trondmy@kernel.org>
Subject: Re: [PATCH 07/10] netfs: Fix missing barriers by using clear_and_wake_up_bit()
Date: Sat, 14 Dec 2024 19:16:18 +0900 [thread overview]
Message-ID: <27fff669-bec4-4255-ba2f-4b154b474d97@gmail.com> (raw)
In-Reply-To: <20241213135013.2964079-8-dhowells@redhat.com>
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(-)
>
[...]
next prev parent reply other threads:[~2024-12-14 10:16 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-13 13:50 [PATCH 00/10] netfs, ceph, nfs, cachefiles: Miscellaneous fixes/changes David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 01/10] kheaders: Ignore silly-rename files David Howells
2024-12-13 13:50 ` David Howells
2024-12-21 5:15 ` Masahiro Yamada
2024-12-21 5:15 ` Masahiro Yamada via Linux-erofs
2024-12-13 13:50 ` [PATCH 02/10] netfs: Fix non-contiguous donation between completed reads David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 03/10] netfs: Fix enomem handling in buffered reads David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 04/10] nfs: Fix oops in nfs_netfs_init_request() when copying to cache David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 05/10] cachefiles: Parse the "secctx" immediately David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 06/10] netfs: Remove redundant use of smp_rmb() David Howells
2024-12-13 13:50 ` David Howells
2024-12-16 10:13 ` Akira Yokosawa
2024-12-16 10:13 ` Akira Yokosawa
2024-12-13 13:50 ` [PATCH 07/10] netfs: Fix missing barriers by using clear_and_wake_up_bit() David Howells
2024-12-13 13:50 ` David Howells
2024-12-14 10:16 ` Akira Yokosawa [this message]
2024-12-14 10:16 ` Akira Yokosawa
2024-12-14 13:44 ` David Howells
2024-12-14 13:44 ` David Howells
2024-12-16 10:11 ` Akira Yokosawa
2024-12-16 10:11 ` Akira Yokosawa
2024-12-13 13:50 ` [PATCH 08/10] netfs: Work around recursion by abandoning retry if nothing read David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 09/10] netfs: Fix ceph copy to cache on write-begin David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 13:50 ` [PATCH 10/10] netfs: Fix the (non-)cancellation of copy when cache is temporarily disabled David Howells
2024-12-13 13:50 ` David Howells
2024-12-13 14:04 ` ceph xfstests failures [was Re: [PATCH 00/10] netfs, ceph, nfs, cachefiles: Miscellaneous fixes/changes] David Howells
2024-12-13 14:04 ` David Howells
2024-12-18 15:10 ` Alex Markuze
2024-12-18 15:10 ` Alex Markuze
2024-12-16 20:34 ` [PATCH 11/10] netfs: Fix is-caching check in read-retry David Howells
2024-12-16 20:34 ` David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27fff669-bec4-4255-ba2f-4b154b474d97@gmail.com \
--to=akiyks@gmail.com \
--cc=ceph-devel@vger.kernel.org \
--cc=christian@brauner.io \
--cc=dhowells@redhat.com \
--cc=idryomov@gmail.com \
--cc=jlayton@kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=max.kellermann@ionos.com \
--cc=netfs@lists.linux.dev \
--cc=trondmy@kernel.org \
--cc=v9fs@lists.linux.dev \
--cc=willy@infradead.org \
--cc=xiubli@redhat.com \
--cc=zilin@seu.edu.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.