raid5: get_active_stripe avoids device_lock

For sequential workload (or request size big workload), get_active_stripe can find cached stripe. In this case, we always hold device_lock, which exposes a lot of lock contention for such workload. If stripe count isn't 0, we don't need hold the lock actually, since we just increase its count. And this is the hot code path for such workload. Unfortunately we must delete the BUG_ON.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/raid5.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +1 -6

 			spin_lock(&conf->device_lock);
-			if (atomic_read(&sh->count)) {
-				BUG_ON(!list_empty(&sh->lru)
-				    && !test_bit(STRIPE_EXPANDING, &sh->state)
-				    && !test_bit(STRIPE_ON_UNPLUG_LIST, &sh->state)
-					);
-			} else {
+			if (!atomic_read(&sh->count)) {
 				if (!test_bit(STRIPE_HANDLE, &sh->state))
 					atomic_inc(&conf->active_stripes);
 				BUG_ON(list_empty(&sh->lru) &&

Quoted 12 lines · On Tue, 8 Apr 2014 12:05:53 +0800 Shaohua Li <[email protected]> wrote:

For sequential workload (or request size big workload), get_active_stripe can find cached stripe. In this case, we always hold device_lock, which exposes a lot of lock contention for such workload. If stripe count isn't 0, we don't need hold the lock actually, since we just increase its count. And this is the hot code path for such workload. Unfortunately we must delete the BUG_ON.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/raid5.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

drivers/md/raid5.c get_active_stripe(struct r5conf *conf, s · line 682

Quoted 10 lines

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +1 -1

@@ -679,14 +679,9 @@ get_active_stripe(struct r5conf *conf, s
 				init_stripe(sh, sector, previous);
 				atomic_inc(&sh->count);
 			}
-		} else {
+		} else if (!atomic_add_unless(&sh->count, 1, 0)) {

Can I be really fussy and ask you to use "atomic_inc_not_zero" rather than "atomic_add_unless" ??
I feel it makes the code a bit clearer.

Quoted 14 lines · NeilBrown

Quoted 12 lines · On Tue, 8 Apr 2014 12:05:53 +0800 Shaohua Li <[email protected]> wrote:

For sequential workload (or request size big workload), get_active_stripe can find cached stripe. In this case, we always hold device_lock, which exposes a lot of lock contention for such workload. If stripe count isn't 0, we don't need hold the lock actually, since we just increase its count. And this is the hot code path for such workload. Unfortunately we must delete the BUG_ON.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/raid5.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Quoted 14 lines

drivers/md/raid5.c get_active_stripe(struct r5conf *conf, s · line 682

Quoted 10 lines

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +1 -1

@@ -679,14 +679,9 @@ get_active_stripe(struct r5conf *conf, s
 				init_stripe(sh, sector, previous);
 				atomic_inc(&sh->count);
 			}
-		} else {
+		} else if (!atomic_add_unless(&sh->count, 1, 0)) {

Can I be really fussy and ask you to use "atomic_inc_not_zero" rather than "atomic_add_unless" ??
I feel it makes the code a bit clearer.

Missed this API. Yes, it's better.

Subject: raid5: get_active_stripe avoids device_lock

For sequential workload (or request size big workload), get_active_stripe can find cached stripe. In this case, we always hold device_lock, which exposes a lot of lock contention for such workload. If stripe count isn't 0, we don't need hold the lock actually, since we just increase its count. And this is the hot code path for such workload. Unfortunately we must delete the BUG_ON.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/raid5.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +1 -1

@@ -679,14 +679,9 @@ get_active_stripe(struct r5conf *conf, s
 				init_stripe(sh, sector, previous);
 				atomic_inc(&sh->count);
 			}
-		} else {
+		} else if (!atomic_inc_not_zero(&sh->count)) {
 			spin_lock(&conf->device_lock);