2023-01-13 12:19:12

by Hou Tao

[permalink] [raw]
Subject: [PATCH v3 0/2] Fixes for fscache volume operations

From: Hou Tao <[email protected]>

Hi,

The patchset includes two fixes for fscache volume operations: patch 1
fixes the hang problem during volume acquisition when the volume
acquisition process waits for the freeing of relinquished volume, patch
2 adds the missing memory barrier in fscache_create_volume_work() and it
is spotted through code review when checking whether or not these is
missing smp_mb() before invoking wake_up_bit().

Comments are always welcome.

Chang Log:
v3:
* Use clear_and_wake_up_bit() helper (Suggested by Jingbo Xu)
* Tidy up commit message and add Reviewed-by tag

v2: https://listman.redhat.com/archives/linux-cachefs/2022-December/007402.html
* rebased on v6.1-rc1
* Patch 1: use wait_on_bit() instead (Suggested by David)
* Patch 2: add the missing smp_mb() in fscache_create_volume_work()

v1: https://listman.redhat.com/archives/linux-cachefs/2022-December/007384.html


Hou Tao (2):
fscache: Use wait_on_bit() to wait for the freeing of relinquished
volume
fscache: Use clear_and_wake_up_bit() in fscache_create_volume_work()

fs/fscache/volume.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

--
2.29.2


2023-01-13 12:22:46

by Hou Tao

[permalink] [raw]
Subject: [PATCH v3 2/2] fscache: Use clear_and_wake_up_bit() in fscache_create_volume_work()

From: Hou Tao <[email protected]>

fscache_create_volume_work() uses wake_up_bit() to wake up the processes
which are waiting for the completion of volume creation. According to
comments in wake_up_bit() and waitqueue_active(), an extra smp_mb() is
needed to guarantee the memory order between FSCACHE_VOLUME_CREATING
flag and waitqueue_active() before invoking wake_up_bit().

Fixing it by using clear_and_wake_up_bit() to add the missing memory
barrier.

Reviewed-by: Jingbo Xu <[email protected]>
Signed-off-by: Hou Tao <[email protected]>
---
fs/fscache/volume.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/fscache/volume.c b/fs/fscache/volume.c
index 903af9d85f8b..cdf991bdd9de 100644
--- a/fs/fscache/volume.c
+++ b/fs/fscache/volume.c
@@ -280,8 +280,7 @@ static void fscache_create_volume_work(struct work_struct *work)
fscache_end_cache_access(volume->cache,
fscache_access_acquire_volume_end);

- clear_bit_unlock(FSCACHE_VOLUME_CREATING, &volume->flags);
- wake_up_bit(&volume->flags, FSCACHE_VOLUME_CREATING);
+ clear_and_wake_up_bit(FSCACHE_VOLUME_CREATING, &volume->flags);
fscache_put_volume(volume, fscache_volume_put_create_work);
}

--
2.29.2

2023-01-13 15:47:47

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] fscache: Use clear_and_wake_up_bit() in fscache_create_volume_work()

On Fri, 2023-01-13 at 19:52 +0800, Hou Tao wrote:
> From: Hou Tao <[email protected]>
>
> fscache_create_volume_work() uses wake_up_bit() to wake up the processes
> which are waiting for the completion of volume creation. According to
> comments in wake_up_bit() and waitqueue_active(), an extra smp_mb() is
> needed to guarantee the memory order between FSCACHE_VOLUME_CREATING
> flag and waitqueue_active() before invoking wake_up_bit().
>
> Fixing it by using clear_and_wake_up_bit() to add the missing memory
> barrier.
>
> Reviewed-by: Jingbo Xu <[email protected]>
> Signed-off-by: Hou Tao <[email protected]>
> ---
> fs/fscache/volume.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/fscache/volume.c b/fs/fscache/volume.c
> index 903af9d85f8b..cdf991bdd9de 100644
> --- a/fs/fscache/volume.c
> +++ b/fs/fscache/volume.c
> @@ -280,8 +280,7 @@ static void fscache_create_volume_work(struct work_struct *work)
> fscache_end_cache_access(volume->cache,
> fscache_access_acquire_volume_end);
>
> - clear_bit_unlock(FSCACHE_VOLUME_CREATING, &volume->flags);
> - wake_up_bit(&volume->flags, FSCACHE_VOLUME_CREATING);
> + clear_and_wake_up_bit(FSCACHE_VOLUME_CREATING, &volume->flags);
> fscache_put_volume(volume, fscache_volume_put_create_work);
> }
>

Reviewed-by: Jeff Layton <[email protected]>

2023-01-30 10:57:19

by Hou Tao

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] Fixes for fscache volume operations

Hi David,

Could you please pick it up for v6.2 ?

On 1/13/2023 7:52 PM, Hou Tao wrote:
> From: Hou Tao <[email protected]>
>
> Hi,
>
> The patchset includes two fixes for fscache volume operations: patch 1
> fixes the hang problem during volume acquisition when the volume
> acquisition process waits for the freeing of relinquished volume, patch
> 2 adds the missing memory barrier in fscache_create_volume_work() and it
> is spotted through code review when checking whether or not these is
> missing smp_mb() before invoking wake_up_bit().
>
> Comments are always welcome.
>
> Chang Log:
> v3:
> * Use clear_and_wake_up_bit() helper (Suggested by Jingbo Xu)
> * Tidy up commit message and add Reviewed-by tag
>
> v2: https://listman.redhat.com/archives/linux-cachefs/2022-December/007402.html
> * rebased on v6.1-rc1
> * Patch 1: use wait_on_bit() instead (Suggested by David)
> * Patch 2: add the missing smp_mb() in fscache_create_volume_work()
>
> v1: https://listman.redhat.com/archives/linux-cachefs/2022-December/007384.html
>
>
> Hou Tao (2):
> fscache: Use wait_on_bit() to wait for the freeing of relinquished
> volume
> fscache: Use clear_and_wake_up_bit() in fscache_create_volume_work()
>
> fs/fscache/volume.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>