raid: replace list_for_each_continue_rcu with new interface

PATCH Applied Michael Wang 3 participants 7 messages Aug 17, 2012 – Sep 11, 2012 1 message not in the archive MD RAID

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michael Wang

author

2012-08-17 04:33 UTC #1 lore raw

PATCH 1/3 raid: replace list_for_each_continue_rcu with new interface

From: Michael Wang <[email protected]>

This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

Signature · 1 line

1.7.4.1

Paul E. McKenney

Reviewed-by on v1 1/3

2012-08-20 23:41 UTC #2 lore raw

Quoted 7 lines · Michael Wang

From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().
Signed-off-by Michael Wang <[email protected]>

Getting rid of list_for_each_continue_rcu() would indeed be a good thing!

Reviewed-by Paul E. McKenney <[email protected]>

Quoted 36 lines

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

Signature · 1 line

1.7.4.1

Michael Wang

author

2012-08-24 00:51 UTC #3 lore raw

In reply to Michael Wang

Quoted 6 lines · Michael Wang

From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().

Hi, Neil

Could I get some comments on this patch?

Regards,
Michael Wang

Quoted 34 lines

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

Michael Wang

author

2012-09-10 08:30 UTC #4 lore raw

Quoted 11 lines · Michael Wang

Quoted 6 lines · Michael Wang
From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().
Hi, Neil
Could I get some comments on this patch?

Hi, Neil

Could I get some comments?

And please forgive and warn me if this patch has came to the wrong place...I get the address from get_maintainer.

Regards,
Michael Wang

Quoted 39 lines

Regards,
Michael Wang

Quoted 34 lines

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

NeilBrown

Acked-by

2012-09-11 06:21 UTC #5 lore raw

Quoted 11 lines · On Mon, 10 Sep 2012 16:30:11 +0800 Michael Wang <[email protected]>…

Quoted 10 lines · Michael Wang
Quoted 6 lines · Michael Wang
From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().
Hi, Neil

Quoted 8 lines

Quoted 1 line
Could I get some comments on this patch?
Hi, Neil
Could I get some comments?
And please forgive and warn me if this patch has came to the wrong place...I get the address from get_maintainer.

Sorry, August was a bad month.

Yes, patch looks good. Shall I include it in my tree, do you want to submit them altogether through some rcu tree?
Either way is fine by me. If you want to submit it through some other tree,

Acked-by NeilBrown <[email protected]>

If not, it'll probably appear in my -next soonish.

Thanks,
NeilBrown

Quoted 43 lines

Regards,
Michael Wang

Quoted 39 lines

Regards,
Michael Wang

Quoted 34 lines

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

Michael Wang

author

2012-09-11 06:27 UTC #6 lore raw

Quoted 23 lines · NeilBrown

Quoted 11 lines · On Mon, 10 Sep 2012 16:30:11 +0800 Michael Wang <[email protected]>…
Quoted 10 lines · Michael Wang
Quoted 6 lines · Michael Wang
From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().
Hi, Neil
Quoted 8 lines
Quoted 1 line
Could I get some comments on this patch?
Hi, Neil
Could I get some comments?
And please forgive and warn me if this patch has came to the wrong place...I get the address from get_maintainer.

Quoted 6 lines

Sorry, August was a bad month.
Yes, patch looks good. Shall I include it in my tree, do you want to submit them altogether through some rcu tree?
Either way is fine by me. If you want to submit it through some other tree,
Acked-by NeilBrown <[email protected]>

Thanks for your review ;-)

I think submit to rcu tree may be better, what's your opinion, Paul?

Regards,
Michael Wang

Quoted 51 lines

If not, it'll probably appear in my -next soonish.

Thanks,
NeilBrown

Quoted 43 lines

Regards,
Michael Wang

Quoted 39 lines

Regards,
Michael Wang

Quoted 34 lines

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */

Paul E. McKenney

2012-09-11 16:28 UTC #7 lore raw

Quoted 26 lines · Michael Wang

Quoted 25 lines · NeilBrown
Quoted 11 lines · On Mon, 10 Sep 2012 16:30:11 +0800 Michael Wang <[email protected]>…
Quoted 10 lines · Michael Wang
Quoted 6 lines · Michael Wang
From: Michael Wang <[email protected]>
This patch replaces list_for_each_continue_rcu() with list_for_each_entry_continue_rcu() to save a few lines of code and allow removing list_for_each_continue_rcu().
Hi, Neil
Quoted 8 lines
Quoted 1 line
Could I get some comments on this patch?
Hi, Neil
Could I get some comments?
And please forgive and warn me if this patch has came to the wrong place...I get the address from get_maintainer.
Sorry, August was a bad month.

Quoted 8 lines

Quoted 4 lines
Yes, patch looks good. Shall I include it in my tree, do you want to submit them altogether through some rcu tree?
Either way is fine by me. If you want to submit it through some other tree,
Acked-by NeilBrown <[email protected]>
Thanks for your review ;-)
I think submit to rcu tree may be better, what's your opinion, Paul?

I am fine either way. If I don't see it in mainline this coming merge window, I will pull it into the -rcu tree.

Thanx, Paul

Quoted 56 lines

Regards,
Michael Wang

Quoted 51 lines

If not, it'll probably appear in my -next soonish.

Thanks,
NeilBrown

Quoted 43 lines

Regards,
Michael Wang

Quoted 39 lines

Regards,
Michael Wang

Quoted 34 lines

Signed-off-by Michael Wang <[email protected]>

---
 drivers/md/bitmap.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

drivers/md/bitmap.c +3 -6

index 15dbe03..b160828 100644
@@ -163,20 +163,17 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde
 	 * As devices are only added or removed when raid_disk is < 0 and
 	 * nr_pending is 0 and In_sync is clear, the entries we return will
 	 * still be in the same position on the list when we re-enter
-	 * list_for_each_continue_rcu.
+	 * list_for_each_entry_continue_rcu.
 	 */
-	struct list_head *pos;
 	rcu_read_lock();
 	if (rdev == NULL)
 		/* start at the beginning */
-		pos = &mddev->disks;
+		rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set);
 	else {
 		/* release the previous rdev and start from there. */
 		rdev_dec_pending(rdev, mddev);
-		pos = &rdev->same_set;
 	}
-	list_for_each_continue_rcu(pos, &mddev->disks) {
-		rdev = list_entry(pos, struct md_rdev, same_set);
+	list_for_each_entry_continue_rcu(rdev, &mddev->disks, same_set) {
 		if (rdev->raid_disk >= 0 &&
 		    !test_bit(Faulty, &rdev->flags)) {
 			/* this is a usable devices */