btrfs: Remove redundant memory barriers

Commit Message

Nikolay Borisov Feb. 14, 2018, 8:53 a.m. UTC

Using any kind of memory barriers around atomic operations which have
a return value is redundant, since those operations themselves are
fully ordered. atomic_t.txt states:

    - RMW operations that have a return value are fully ordered;

    Fully ordered primitives are ordered against everything prior and
    everything subsequent. Therefore a fully ordered primitive is like
    having an smp_mb() before and an smp_mb() after the primitive.

Given this let's replace the extra memory barriers with comments.

Signed-off-by: Nikolay Borisov <[e-mail]>
---
 fs/btrfs/inode.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

David Sterba Feb. 23, 2018, 11:42 p.m. UTC | #1

On Wed, Feb 14, 2018 at 10:53:36AM +0200, Nikolay Borisov wrote:
> Using any kind of memory barriers around atomic operations which have
> a return value is redundant, since those operations themselves are
> fully ordered. atomic_t.txt states:
> 
>     - RMW operations that have a return value are fully ordered;
> 
>     Fully ordered primitives are ordered against everything prior and
>     everything subsequent. Therefore a fully ordered primitive is like
>     having an smp_mb() before and an smp_mb() after the primitive.
> 
> Given this let's replace the extra memory barriers with comments.
> 
> Signed-off-by: Nikolay Borisov <[e-mail]>

Reviewed-by: David Sterba <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8997e39c8e37..b6e4288010fa 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8321,13 +8321,13 @@  static void btrfs_end_dio_bio(struct bio *bio)
 		err = dip->subio_endio(dip->inode, btrfs_io_bio(bio), err);
 
 	if (err) {
-		dip->errors = 1;
-
 		/*
-		 * before atomic variable goto zero, we must make sure
-		 * dip->errors is perceived to be set.
+		 * We want to perceive the errors flag being set before
+		 * decrementing the reference count. We don't need a barrier
+		 * since atomic operations with a return value are fully
+		 * ordered as per atomic_t.txt
 		 */
-		smp_mb__before_atomic();
+		dip->errors = 1;
 	}
 
 	/* if there are more bios still pending for this dio, just exit */
@@ -8515,10 +8515,11 @@  static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
 out_err:
 	dip->errors = 1;
 	/*
-	 * before atomic variable goto zero, we must
-	 * make sure dip->errors is perceived to be set.
+	 * Before atomic variable goto zero, we must  make sure dip->errors is
+	 * perceived to be set. This ordering is ensured by the fact that an
+	 * atomic operations with a return value are fully ordered as per
+	 * atomic_t.txt
 	 */
-	smp_mb__before_atomic();
 	if (atomic_dec_and_test(&dip->pending_bios))
 		bio_io_error(dip->orig_bio);