* [PATCH v2] staging: lustre: lnet: Use list_for_each_entry
@ 2016-02-26 10:00 Bhaktipriya Shridhar
2016-02-26 10:06 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 3+ messages in thread
From: Bhaktipriya Shridhar @ 2016-02-26 10:00 UTC (permalink / raw
To: outreachy-kernel
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);
...
}
Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
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(-)
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index efb7169..8c1d50a7 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: lustre: lnet: Use list_for_each_entry
2016-02-26 10:00 [PATCH v2] staging: lustre: lnet: Use list_for_each_entry Bhaktipriya Shridhar
@ 2016-02-26 10:06 ` Julia Lawall
2016-02-27 17:39 ` Bhaktipriya Shridhar
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2016-02-26 10:06 UTC (permalink / raw
To: Bhaktipriya Shridhar; +Cc: outreachy-kernel
On Fri, 26 Feb 2016, Bhaktipriya Shridhar wrote:
> 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
> 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);
> ...
> }
>
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> index efb7169..8c1d50a7 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: lustre: lnet: Use list_for_each_entry
2016-02-26 10:06 ` [Outreachy kernel] " Julia Lawall
@ 2016-02-27 17:39 ` Bhaktipriya Shridhar
0 siblings, 0 replies; 3+ messages in thread
From: Bhaktipriya Shridhar @ 2016-02-27 17:39 UTC (permalink / raw
To: Julia Lawall; +Cc: outreachy-kernel
Hello Julia,
I'll send in a v3 with a changed commit message.
Thanks,
Bhaktipriya
On Fri, Feb 26, 2016 at 3:36 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
> On Fri, 26 Feb 2016, Bhaktipriya Shridhar wrote:
>
>> 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
>
>> 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);
>> ...
>> }
>>
>> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>> index efb7169..8c1d50a7 100644
>> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>> @@ -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 outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> 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.
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-27 17:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-26 10:00 [PATCH v2] staging: lustre: lnet: Use list_for_each_entry Bhaktipriya Shridhar
2016-02-26 10:06 ` [Outreachy kernel] " Julia Lawall
2016-02-27 17:39 ` Bhaktipriya Shridhar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.