Skip to contentSkip to conversation

staging: lustre: lnet: Use list_for_each_entry

PATCH Superseded Bhaktipriya Shridhar 2 participants 3 messages Feb 26, 2016 – Feb 27, 2016 Staging

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 staging: lustre: lnet: Use list_for_each_entry

In general, doubly linked lists are iterated using list_empty and list_entry functions. But it can be better written using list_for_each_entry macro.

This patch replaces the while loop containing list_empty and list_entry with list_for_each_entry and list_for_each_entry_safe(if list_del is used int the loop).

This was done with Coccinelle.

@@
expression E1;
identifier I1, I2;
type T;
iterator name list_for_each_entry_safe;
@@

T *I1;
+ T *temp;
...
- while (list_empty(&E1) == 0)
+ list_for_each_entry_safe (I1, temp, &E1, I2)
{
...when != T *I1;
- I1 = list_entry(E1.next, T, I2);
...
}

---
Changes in v2:
-Fixed typo "lusture" to "lustre" in the subject.

 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c +2 -3
index efb7169..8c1d50a7 100644
@@ -2354,13 +2354,12 @@ static inline void
 ksocknal_flush_stale_txs(ksock_peer_t *peer)
 {
 	ksock_tx_t *tx;
+	ksock_tx_t *temp;
 	LIST_HEAD(stale_txs);
 
 	write_lock_bh(&ksocknal_data.ksnd_global_lock);
 
-	while (!list_empty(&peer->ksnp_tx_queue)) {
-		tx = list_entry(peer->ksnp_tx_queue.next, ksock_tx_t, tx_list);
-
+	list_for_each_entry_safe(tx, temp, &peer->ksnp_tx_queue, tx_list) {
 		if (!cfs_time_aftereq(cfs_time_current(),
 				      tx->tx_deadline))
 			break;

--
2.1.4

#1 lore raw
Quoted 2 lines · Bhaktipriya Shridhar

In general, doubly linked lists are iterated using list_empty and list_entry functions.

It's a little bit fussy, but this is not true. I had neer seen this pattern before you found it. So I don't think that it is the general case. Currently there are over 2000 uses of list_for_each_entry.

Also the subject and this part of the commit message mention list_for_each_entry, but actually you are using list_for_each_entry_safe. And list_for_each_entry_safe is the only thing that makes sense when there is a list_empty test on the loop, because for the list to be empty, you have to remove things from it, and if you remove things from it, you need to use list_for_each_entry_safe.

Note that when you reach the "end" of a doubly linked list, you actually reach the beginning, not a NULL element like for a singly linked list, so reaching the end of the list does not make list_empty true.

julia

Quoted 65 lines

But it can be better written using
list_for_each_entry macro.

This patch replaces the while loop containing list_empty and list_entry with list_for_each_entry and list_for_each_entry_safe(if list_del is used int the loop).

This was done with Coccinelle.

@@
expression E1;
identifier I1, I2;
type T;
iterator name list_for_each_entry_safe;
@@

T *I1;
+ T *temp;
...
- while (list_empty(&E1) == 0)
+ list_for_each_entry_safe (I1, temp, &E1, I2)
{
...when != T *I1;
- I1 = list_entry(E1.next, T, I2);
...
}

---
Changes in v2:
-Fixed typo "lusture" to "lustre" in the subject.

 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c +2 -4
index efb7169..8c1d50a7 100644
@@ -2354,13 +2354,12 @@ static inline void
 ksocknal_flush_stale_txs(ksock_peer_t *peer)
 {
 	ksock_tx_t *tx;
+	ksock_tx_t *temp;
 	LIST_HEAD(stale_txs);
 
 	write_lock_bh(&ksocknal_data.ksnd_global_lock);
 
-	while (!list_empty(&peer->ksnp_tx_queue)) {
-		tx = list_entry(peer->ksnp_tx_queue.next, ksock_tx_t, tx_list);
-
+	list_for_each_entry_safe(tx, temp, &peer->ksnp_tx_queue, tx_list) {
 		if (!cfs_time_aftereq(cfs_time_current(),
 				      tx->tx_deadline))
 			break;
--

2.1.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160226100041.GA11042%40Karyakshetra.
For more options, visit https://groups.google.com/d/optout.

#2 lore raw

Hello Julia,

I'll send in a v3 with a changed commit message.

Thanks,
Bhaktipriya

Quoted 88 lines · Julia Lawall
Quoted 2 lines · Bhaktipriya Shridhar

In general, doubly linked lists are iterated using list_empty and list_entry functions.

It's a little bit fussy, but this is not true. I had neer seen this pattern before you found it. So I don't think that it is the general case. Currently there are over 2000 uses of list_for_each_entry.

Also the subject and this part of the commit message mention list_for_each_entry, but actually you are using list_for_each_entry_safe. And list_for_each_entry_safe is the only thing that makes sense when there is a list_empty test on the loop, because for the list to be empty, you have to remove things from it, and if you remove things from it, you need to use list_for_each_entry_safe.

Note that when you reach the "end" of a doubly linked list, you actually reach the beginning, not a NULL element like for a singly linked list, so reaching the end of the list does not make list_empty true.

julia

Quoted 65 lines

But it can be better written using
list_for_each_entry macro.

This patch replaces the while loop containing list_empty and list_entry with list_for_each_entry and list_for_each_entry_safe(if list_del is used int the loop).

This was done with Coccinelle.

@@
expression E1;
identifier I1, I2;
type T;
iterator name list_for_each_entry_safe;
@@

T *I1;
+ T *temp;
...
- while (list_empty(&E1) == 0)
+ list_for_each_entry_safe (I1, temp, &E1, I2)
{
...when != T *I1;
- I1 = list_entry(E1.next, T, I2);
...
}

---
Changes in v2:
-Fixed typo "lusture" to "lustre" in the subject.

 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c +2 -4
index efb7169..8c1d50a7 100644
@@ -2354,13 +2354,12 @@ static inline void
 ksocknal_flush_stale_txs(ksock_peer_t *peer)
 {
      ksock_tx_t *tx;
+     ksock_tx_t *temp;
      LIST_HEAD(stale_txs);
 
      write_lock_bh(&ksocknal_data.ksnd_global_lock);
 
-     while (!list_empty(&peer->ksnp_tx_queue)) {
-             tx = list_entry(peer->ksnp_tx_queue.next, ksock_tx_t, tx_list);
-
+     list_for_each_entry_safe(tx, temp, &peer->ksnp_tx_queue, tx_list) {
              if (!cfs_time_aftereq(cfs_time_current(),
                                    tx->tx_deadline))
                      break;
--

2.1.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160226100041.GA11042%40Karyakshetra.
For more options, visit https://groups.google.com/d/optout.