[PATCH v7 02/14] fs: spell out fenced ->i_state accesses with explicit smp_wmb/smp_rmb

Mateusz Guzik posted 14 patches 2 months, 1 week ago
[PATCH v7 02/14] fs: spell out fenced ->i_state accesses with explicit smp_wmb/smp_rmb
Posted by Mateusz Guzik 2 months, 1 week ago
The incomming helpers don't ship with _release/_acquire variants, for
the time being anyway.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/fs-writeback.c           | 5 +++--
 include/linux/backing-dev.h | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 2b35e80037fe..9cda19a40ca2 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -476,10 +476,11 @@ static bool inode_do_switch_wbs(struct inode *inode,
 	switched = true;
 skip_switch:
 	/*
-	 * Paired with load_acquire in unlocked_inode_to_wb_begin() and
+	 * Paired with an acquire fence in unlocked_inode_to_wb_begin() and
 	 * ensures that the new wb is visible if they see !I_WB_SWITCH.
 	 */
-	smp_store_release(&inode->i_state, inode->i_state & ~I_WB_SWITCH);
+	smp_wmb();
+	inode->i_state &= ~I_WB_SWITCH;
 
 	xa_unlock_irq(&mapping->i_pages);
 	spin_unlock(&inode->i_lock);
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 3e64f14739dd..065cba5dc111 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -277,10 +277,11 @@ unlocked_inode_to_wb_begin(struct inode *inode, struct wb_lock_cookie *cookie)
 	rcu_read_lock();
 
 	/*
-	 * Paired with store_release in inode_switch_wbs_work_fn() and
+	 * Paired with a release fence in inode_do_switch_wbs() and
 	 * ensures that we see the new wb if we see cleared I_WB_SWITCH.
 	 */
-	cookie->locked = smp_load_acquire(&inode->i_state) & I_WB_SWITCH;
+	cookie->locked = inode->i_state & I_WB_SWITCH;
+	smp_rmb();
 
 	if (unlikely(cookie->locked))
 		xa_lock_irqsave(&inode->i_mapping->i_pages, cookie->flags);
-- 
2.34.1
Re: [PATCH v7 02/14] fs: spell out fenced ->i_state accesses with explicit smp_wmb/smp_rmb
Posted by Jan Kara 2 months, 1 week ago
On Thu 09-10-25 09:59:16, Mateusz Guzik wrote:
> The incomming helpers don't ship with _release/_acquire variants, for
> the time being anyway.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/fs-writeback.c           | 5 +++--
>  include/linux/backing-dev.h | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 2b35e80037fe..9cda19a40ca2 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -476,10 +476,11 @@ static bool inode_do_switch_wbs(struct inode *inode,
>  	switched = true;
>  skip_switch:
>  	/*
> -	 * Paired with load_acquire in unlocked_inode_to_wb_begin() and
> +	 * Paired with an acquire fence in unlocked_inode_to_wb_begin() and
>  	 * ensures that the new wb is visible if they see !I_WB_SWITCH.
>  	 */
> -	smp_store_release(&inode->i_state, inode->i_state & ~I_WB_SWITCH);
> +	smp_wmb();
> +	inode->i_state &= ~I_WB_SWITCH;
>  
>  	xa_unlock_irq(&mapping->i_pages);
>  	spin_unlock(&inode->i_lock);
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 3e64f14739dd..065cba5dc111 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -277,10 +277,11 @@ unlocked_inode_to_wb_begin(struct inode *inode, struct wb_lock_cookie *cookie)
>  	rcu_read_lock();
>  
>  	/*
> -	 * Paired with store_release in inode_switch_wbs_work_fn() and
> +	 * Paired with a release fence in inode_do_switch_wbs() and
>  	 * ensures that we see the new wb if we see cleared I_WB_SWITCH.
>  	 */
> -	cookie->locked = smp_load_acquire(&inode->i_state) & I_WB_SWITCH;
> +	cookie->locked = inode->i_state & I_WB_SWITCH;
> +	smp_rmb();
>  
>  	if (unlikely(cookie->locked))
>  		xa_lock_irqsave(&inode->i_mapping->i_pages, cookie->flags);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR