Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1592797 > unrolled thread
| Started by | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| First post | 2017-03-05 18:20 +0100 |
| Last post | 2017-03-07 13:10 +0100 |
| Articles | 13 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH 0/5] Use list_for_each_entry_safe simran singhal <singhalsimran0@gmail.com> - 2017-03-05 18:20 +0100
[PATCH 4/5] staging: lustre: llite: Use list_for_each_entry_safe simran singhal <singhalsimran0@gmail.com> - 2017-03-05 18:20 +0100
[PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe simran singhal <singhalsimran0@gmail.com> - 2017-03-05 18:20 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe Julia Lawall <julia.lawall@lip6.fr> - 2017-03-05 18:20 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe Julia Lawall <julia.lawall@lip6.fr> - 2017-03-05 18:30 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe Julia Lawall <julia.lawall@lip6.fr> - 2017-03-05 18:30 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe Julia Lawall <julia.lawall@lip6.fr> - 2017-03-05 18:50 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-03-05 19:00 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe Julia Lawall <julia.lawall@lip6.fr> - 2017-03-05 19:00 +0100
Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-03-05 18:50 +0100
Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe James Simmons <jsimmons@infradead.org> - 2017-03-06 16:30 +0100
Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-03-06 20:30 +0100
Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe "Dilger, Andreas" <andreas.dilger@intel.com> - 2017-03-07 13:10 +0100
| From | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-05 18:20 +0100 |
| Subject | [PATCH 0/5] Use list_for_each_entry_safe |
| Message-ID | <thHZ8-3pL-9@gated-at.bofh.it> |
This patch-series replaces the while loop containing list_empty and list_entry with list_for_each_entry_safe. simran singhal (5): staging: lustre: Use list_for_each_entry_safe staging: lustre: ptlrpc: Use list_for_each_entry_safe staging: lustre: osc: Use list_for_each_entry_safe staging: lustre: llite: Use list_for_each_entry_safe staging: lustre: osc_page.c: Use list_for_each_entry_safe drivers/staging/lustre/lustre/llite/statahead.c | 5 ++--- drivers/staging/lustre/lustre/osc/osc_cache.c | 5 ++--- drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++------- drivers/staging/lustre/lustre/ptlrpc/sec_gc.c | 5 ++--- drivers/staging/lustre/lustre/ptlrpc/service.c | 5 ++--- 5 files changed, 12 insertions(+), 19 deletions(-) -- 2.7.4
[toc] | [next] | [standalone]
| From | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-05 18:20 +0100 |
| Subject | [PATCH 4/5] staging: lustre: llite: Use list_for_each_entry_safe |
| Message-ID | <thHZ8-3pL-19@gated-at.bofh.it> |
| In reply to | #1592797 |
Doubly linked lists which are iterated using list_empty
and list_entry macros have been replaced with list_for_each_entry_safe
macro.
This makes the iteration simpler and more readable.
This patch replaces the while loop containing list_empty and list_entry
with list_for_each_entry_safe.
This was done with Coccinelle.
@@
expression E1;
identifier I1, I2;
type T;
iterator name list_for_each_entry_safe;
@@
T *I1;
+ T *tmp;
...
- while (list_empty(&E1) == 0)
+ list_for_each_entry_safe (I1, tmp, &E1, I2)
{
...when != T *I1;
- I1 = list_entry(E1.next, T, I2);
...
}
Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
drivers/staging/lustre/lustre/llite/statahead.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c
index fb7c315..d1287b2 100644
--- a/drivers/staging/lustre/lustre/llite/statahead.c
+++ b/drivers/staging/lustre/lustre/llite/statahead.c
@@ -860,6 +860,7 @@ static int ll_agl_thread(void *arg)
struct inode *dir = d_inode(parent);
struct ll_inode_info *plli = ll_i2info(dir);
struct ll_inode_info *clli;
+ struct ll_inode_info *tmp;
struct ll_sb_info *sbi = ll_i2sbi(dir);
struct ll_statahead_info *sai;
struct ptlrpc_thread *thread;
@@ -909,9 +910,7 @@ static int ll_agl_thread(void *arg)
spin_lock(&plli->lli_agl_lock);
sai->sai_agl_valid = 0;
- while (!list_empty(&sai->sai_agls)) {
- clli = list_entry(sai->sai_agls.next,
- struct ll_inode_info, lli_agl_list);
+ list_for_each_entry_safe(clli, tmp, &sai->sai_agls, lli_agl_list) {
list_del_init(&clli->lli_agl_list);
spin_unlock(&plli->lli_agl_lock);
clli->lli_agl_index = 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | simran singhal <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-05 18:20 +0100 |
| Subject | [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thHZ8-3pL-21@gated-at.bofh.it> |
| In reply to | #1592797 |
Doubly linked lists which are iterated using list_empty
and list_entry macros have been replaced with list_for_each_entry_safe
macro.
This makes the iteration simpler and more readable.
This patch replaces the while loop containing list_empty and list_entry
with list_for_each_entry_safe.
This was done with Coccinelle.
@@
expression E1;
identifier I1, I2;
type T;
iterator name list_for_each_entry_safe;
@@
T *I1;
+ T *tmp;
...
- while (list_empty(&E1) == 0)
+ list_for_each_entry_safe (I1, tmp, &E1, I2)
{
...when != T *I1;
- I1 = list_entry(E1.next, T, I2);
...
}
Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
index ed8a0dc..e8b974f 100644
--- a/drivers/staging/lustre/lustre/osc/osc_page.c
+++ b/drivers/staging/lustre/lustre/osc/osc_page.c
@@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
struct cl_object *clobj = NULL;
struct cl_page **pvec;
struct osc_page *opg;
+ struct osc_page *tmp;
int maxscan = 0;
long count = 0;
int index = 0;
@@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
if (force)
cli->cl_lru_reclaim++;
maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
- while (!list_empty(&cli->cl_lru_list)) {
+ list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
struct cl_page *page;
bool will_free = false;
@@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
if (--maxscan < 0)
break;
- opg = list_entry(cli->cl_lru_list.next, struct osc_page,
- ops_lru);
page = opg->ops_cl.cpl_page;
if (lru_page_busy(cli, page)) {
list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
@@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
{
struct client_obd *stop_anchor = NULL;
struct client_obd *cli;
+ struct client_obd *tmp;
struct lu_env *env;
long shrank = 0;
u16 refcheck;
@@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
return SHRINK_STOP;
spin_lock(&osc_shrink_lock);
- while (!list_empty(&osc_shrink_list)) {
- cli = list_entry(osc_shrink_list.next, struct client_obd,
- cl_shrink_list);
-
+ list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
if (!stop_anchor)
stop_anchor = cli;
else if (cli == stop_anchor)
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-03-05 18:20 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thHZ8-3pL-23@gated-at.bofh.it> |
| In reply to | #1592799 |
On Sun, 5 Mar 2017, simran singhal wrote:
> Doubly linked lists which are iterated using list_empty
> and list_entry macros have been replaced with list_for_each_entry_safe
> macro.
> This makes the iteration simpler and more readable.
>
> This patch replaces the while loop containing list_empty and list_entry
> with list_for_each_entry_safe.
list_for_each_entry_safe is only needed when the current element is
removed from the list within the loop body. If that is not the case, you
can just use list_for_each_entry.
julia
>
> This was done with Coccinelle.
>
> @@
> expression E1;
> identifier I1, I2;
> type T;
> iterator name list_for_each_entry_safe;
> @@
>
> T *I1;
> + T *tmp;
> ...
> - while (list_empty(&E1) == 0)
> + list_for_each_entry_safe (I1, tmp, &E1, I2)
> {
> ...when != T *I1;
> - I1 = list_entry(E1.next, T, I2);
> ...
> }
>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> index ed8a0dc..e8b974f 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> struct cl_object *clobj = NULL;
> struct cl_page **pvec;
> struct osc_page *opg;
> + struct osc_page *tmp;
> int maxscan = 0;
> long count = 0;
> int index = 0;
> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (force)
> cli->cl_lru_reclaim++;
> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> - while (!list_empty(&cli->cl_lru_list)) {
> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> struct cl_page *page;
> bool will_free = false;
>
> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (--maxscan < 0)
> break;
>
> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> - ops_lru);
> page = opg->ops_cl.cpl_page;
> if (lru_page_busy(cli, page)) {
> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> {
> struct client_obd *stop_anchor = NULL;
> struct client_obd *cli;
> + struct client_obd *tmp;
> struct lu_env *env;
> long shrank = 0;
> u16 refcheck;
> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> return SHRINK_STOP;
>
> spin_lock(&osc_shrink_lock);
> - while (!list_empty(&osc_shrink_list)) {
> - cli = list_entry(osc_shrink_list.next, struct client_obd,
> - cl_shrink_list);
> -
> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> if (!stop_anchor)
> stop_anchor = cli;
> else if (cli == stop_anchor)
> --
> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-03-05 18:30 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thI8O-3uS-15@gated-at.bofh.it> |
| In reply to | #1592800 |
On Sun, 5 Mar 2017, Julia Lawall wrote:
>
>
> On Sun, 5 Mar 2017, simran singhal wrote:
>
> > Doubly linked lists which are iterated using list_empty
> > and list_entry macros have been replaced with list_for_each_entry_safe
> > macro.
> > This makes the iteration simpler and more readable.
> >
> > This patch replaces the while loop containing list_empty and list_entry
> > with list_for_each_entry_safe.
>
> list_for_each_entry_safe is only needed when the current element is
> removed from the list within the loop body. If that is not the case, you
> can just use list_for_each_entry.
Sorry, my comment was likely completely off the mark here. I was thinking
that the original code was using list_for_each. With the while
(!list_empty pattern, the safe version is definitely needed.
julia
>
> julia
>
>
> >
> > This was done with Coccinelle.
> >
> > @@
> > expression E1;
> > identifier I1, I2;
> > type T;
> > iterator name list_for_each_entry_safe;
> > @@
> >
> > T *I1;
> > + T *tmp;
> > ...
> > - while (list_empty(&E1) == 0)
> > + list_for_each_entry_safe (I1, tmp, &E1, I2)
> > {
> > ...when != T *I1;
> > - I1 = list_entry(E1.next, T, I2);
> > ...
> > }
> >
> > Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> > ---
> > drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> > index ed8a0dc..e8b974f 100644
> > --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> > +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> > @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> > struct cl_object *clobj = NULL;
> > struct cl_page **pvec;
> > struct osc_page *opg;
> > + struct osc_page *tmp;
> > int maxscan = 0;
> > long count = 0;
> > int index = 0;
> > @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> > if (force)
> > cli->cl_lru_reclaim++;
> > maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> > - while (!list_empty(&cli->cl_lru_list)) {
> > + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> > struct cl_page *page;
> > bool will_free = false;
> >
> > @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> > if (--maxscan < 0)
> > break;
> >
> > - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> > - ops_lru);
> > page = opg->ops_cl.cpl_page;
> > if (lru_page_busy(cli, page)) {
> > list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> > @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> > {
> > struct client_obd *stop_anchor = NULL;
> > struct client_obd *cli;
> > + struct client_obd *tmp;
> > struct lu_env *env;
> > long shrank = 0;
> > u16 refcheck;
> > @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> > return SHRINK_STOP;
> >
> > spin_lock(&osc_shrink_lock);
> > - while (!list_empty(&osc_shrink_list)) {
> > - cli = list_entry(osc_shrink_list.next, struct client_obd,
> > - cl_shrink_list);
> > -
> > + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> > if (!stop_anchor)
> > stop_anchor = cli;
> > else if (cli == stop_anchor)
> > --
> > 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-03-05 18:30 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thI8O-3uS-5@gated-at.bofh.it> |
| In reply to | #1592799 |
By the way, the above subject line is not correct. Normally one does not
put .c/.h in the subject line. Indeed, there is not really any
deterministic algorithm for choosing the subject line. You need to run
git log --oneline filename to see what others have done.
julia
On Sun, 5 Mar 2017, simran singhal wrote:
> Doubly linked lists which are iterated using list_empty
> and list_entry macros have been replaced with list_for_each_entry_safe
> macro.
> This makes the iteration simpler and more readable.
>
> This patch replaces the while loop containing list_empty and list_entry
> with list_for_each_entry_safe.
>
> This was done with Coccinelle.
>
> @@
> expression E1;
> identifier I1, I2;
> type T;
> iterator name list_for_each_entry_safe;
> @@
>
> T *I1;
> + T *tmp;
> ...
> - while (list_empty(&E1) == 0)
> + list_for_each_entry_safe (I1, tmp, &E1, I2)
> {
> ...when != T *I1;
> - I1 = list_entry(E1.next, T, I2);
> ...
> }
>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> ---
> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> index ed8a0dc..e8b974f 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> struct cl_object *clobj = NULL;
> struct cl_page **pvec;
> struct osc_page *opg;
> + struct osc_page *tmp;
> int maxscan = 0;
> long count = 0;
> int index = 0;
> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (force)
> cli->cl_lru_reclaim++;
> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> - while (!list_empty(&cli->cl_lru_list)) {
> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> struct cl_page *page;
> bool will_free = false;
>
> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (--maxscan < 0)
> break;
>
> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> - ops_lru);
> page = opg->ops_cl.cpl_page;
> if (lru_page_busy(cli, page)) {
> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> {
> struct client_obd *stop_anchor = NULL;
> struct client_obd *cli;
> + struct client_obd *tmp;
> struct lu_env *env;
> long shrank = 0;
> u16 refcheck;
> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> return SHRINK_STOP;
>
> spin_lock(&osc_shrink_lock);
> - while (!list_empty(&osc_shrink_list)) {
> - cli = list_entry(osc_shrink_list.next, struct client_obd,
> - cl_shrink_list);
> -
> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> if (!stop_anchor)
> stop_anchor = cli;
> else if (cli == stop_anchor)
> --
> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-03-05 18:50 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thIs9-3HM-11@gated-at.bofh.it> |
| In reply to | #1592804 |
On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > By the way, the above subject line is not correct. Normally one does not
> > put .c/.h in the subject line. Indeed, there is not really any
> > deterministic algorithm for choosing the subject line. You need to run
> > git log --oneline filename to see what others have done.
>
> So this would be fine Subject:-
> staging: lustre: osc_page: Use list_for_each_entry_safe
Looking at the result of git log --oneline, I would say:
staging: lustre: osc:
For example that it what is used by 86df598, which also only changes the
file modified by this patch.
julia
>
> >
> > julia
> >
> > On Sun, 5 Mar 2017, simran singhal wrote:
> >
> >> Doubly linked lists which are iterated using list_empty
> >> and list_entry macros have been replaced with list_for_each_entry_safe
> >> macro.
> >> This makes the iteration simpler and more readable.
> >>
> >> This patch replaces the while loop containing list_empty and list_entry
> >> with list_for_each_entry_safe.
> >>
> >> This was done with Coccinelle.
> >>
> >> @@
> >> expression E1;
> >> identifier I1, I2;
> >> type T;
> >> iterator name list_for_each_entry_safe;
> >> @@
> >>
> >> T *I1;
> >> + T *tmp;
> >> ...
> >> - while (list_empty(&E1) == 0)
> >> + list_for_each_entry_safe (I1, tmp, &E1, I2)
> >> {
> >> ...when != T *I1;
> >> - I1 = list_entry(E1.next, T, I2);
> >> ...
> >> }
> >>
> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> >> ---
> >> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> >> 1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> >> index ed8a0dc..e8b974f 100644
> >> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> >> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> >> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> struct cl_object *clobj = NULL;
> >> struct cl_page **pvec;
> >> struct osc_page *opg;
> >> + struct osc_page *tmp;
> >> int maxscan = 0;
> >> long count = 0;
> >> int index = 0;
> >> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> if (force)
> >> cli->cl_lru_reclaim++;
> >> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> >> - while (!list_empty(&cli->cl_lru_list)) {
> >> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> >> struct cl_page *page;
> >> bool will_free = false;
> >>
> >> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> if (--maxscan < 0)
> >> break;
> >>
> >> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> >> - ops_lru);
> >> page = opg->ops_cl.cpl_page;
> >> if (lru_page_busy(cli, page)) {
> >> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> >> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> >> {
> >> struct client_obd *stop_anchor = NULL;
> >> struct client_obd *cli;
> >> + struct client_obd *tmp;
> >> struct lu_env *env;
> >> long shrank = 0;
> >> u16 refcheck;
> >> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> >> return SHRINK_STOP;
> >>
> >> spin_lock(&osc_shrink_lock);
> >> - while (!list_empty(&osc_shrink_list)) {
> >> - cli = list_entry(osc_shrink_list.next, struct client_obd,
> >> - cl_shrink_list);
> >> -
> >> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> >> if (!stop_anchor)
> >> stop_anchor = cli;
> >> else if (cli == stop_anchor)
> >> --
> >> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
[toc] | [prev] | [next] | [standalone]
| From | SIMRAN SINGHAL <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-05 19:00 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thIBP-3Lv-3@gated-at.bofh.it> |
| In reply to | #1592810 |
On Sun, Mar 5, 2017 at 11:18 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
>
>> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > By the way, the above subject line is not correct. Normally one does not
>> > put .c/.h in the subject line. Indeed, there is not really any
>> > deterministic algorithm for choosing the subject line. You need to run
>> > git log --oneline filename to see what others have done.
>>
>> So this would be fine Subject:-
>> staging: lustre: osc_page: Use list_for_each_entry_safe
>
> Looking at the result of git log --oneline, I would say:
>
> staging: lustre: osc:
>
> For example that it what is used by 86df598, which also only changes the
> file modified by this patch.
>
Actually, I can't use this as its already subject of one of a patch of
this patch
series.
> julia
>
>>
>> >
>> > julia
>> >
>> > On Sun, 5 Mar 2017, simran singhal wrote:
>> >
>> >> Doubly linked lists which are iterated using list_empty
>> >> and list_entry macros have been replaced with list_for_each_entry_safe
>> >> macro.
>> >> This makes the iteration simpler and more readable.
>> >>
>> >> This patch replaces the while loop containing list_empty and list_entry
>> >> with list_for_each_entry_safe.
>> >>
>> >> This was done with Coccinelle.
>> >>
>> >> @@
>> >> expression E1;
>> >> identifier I1, I2;
>> >> type T;
>> >> iterator name list_for_each_entry_safe;
>> >> @@
>> >>
>> >> T *I1;
>> >> + T *tmp;
>> >> ...
>> >> - while (list_empty(&E1) == 0)
>> >> + list_for_each_entry_safe (I1, tmp, &E1, I2)
>> >> {
>> >> ...when != T *I1;
>> >> - I1 = list_entry(E1.next, T, I2);
>> >> ...
>> >> }
>> >>
>> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> >> ---
>> >> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
>> >> 1 file changed, 4 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
>> >> index ed8a0dc..e8b974f 100644
>> >> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
>> >> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
>> >> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> >> struct cl_object *clobj = NULL;
>> >> struct cl_page **pvec;
>> >> struct osc_page *opg;
>> >> + struct osc_page *tmp;
>> >> int maxscan = 0;
>> >> long count = 0;
>> >> int index = 0;
>> >> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> >> if (force)
>> >> cli->cl_lru_reclaim++;
>> >> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
>> >> - while (!list_empty(&cli->cl_lru_list)) {
>> >> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
>> >> struct cl_page *page;
>> >> bool will_free = false;
>> >>
>> >> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> >> if (--maxscan < 0)
>> >> break;
>> >>
>> >> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
>> >> - ops_lru);
>> >> page = opg->ops_cl.cpl_page;
>> >> if (lru_page_busy(cli, page)) {
>> >> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
>> >> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> >> {
>> >> struct client_obd *stop_anchor = NULL;
>> >> struct client_obd *cli;
>> >> + struct client_obd *tmp;
>> >> struct lu_env *env;
>> >> long shrank = 0;
>> >> u16 refcheck;
>> >> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> >> return SHRINK_STOP;
>> >>
>> >> spin_lock(&osc_shrink_lock);
>> >> - while (!list_empty(&osc_shrink_list)) {
>> >> - cli = list_entry(osc_shrink_list.next, struct client_obd,
>> >> - cl_shrink_list);
>> >> -
>> >> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
>> >> if (!stop_anchor)
>> >> stop_anchor = cli;
>> >> else if (cli == stop_anchor)
>> >> --
>> >> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-03-05 19:00 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thIBP-3Lv-19@gated-at.bofh.it> |
| In reply to | #1592817 |
On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> On Sun, Mar 5, 2017 at 11:18 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> >
> >> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> > By the way, the above subject line is not correct. Normally one does not
> >> > put .c/.h in the subject line. Indeed, there is not really any
> >> > deterministic algorithm for choosing the subject line. You need to run
> >> > git log --oneline filename to see what others have done.
> >>
> >> So this would be fine Subject:-
> >> staging: lustre: osc_page: Use list_for_each_entry_safe
> >
> > Looking at the result of git log --oneline, I would say:
> >
> > staging: lustre: osc:
> >
> > For example that it what is used by 86df598, which also only changes the
> > file modified by this patch.
> >
> Actually, I can't use this as its already subject of one of a patch of
> this patch
> series.
OK, then what you suggested would be ok.
julia
>
> > julia
> >
> >>
> >> >
> >> > julia
> >> >
> >> > On Sun, 5 Mar 2017, simran singhal wrote:
> >> >
> >> >> Doubly linked lists which are iterated using list_empty
> >> >> and list_entry macros have been replaced with list_for_each_entry_safe
> >> >> macro.
> >> >> This makes the iteration simpler and more readable.
> >> >>
> >> >> This patch replaces the while loop containing list_empty and list_entry
> >> >> with list_for_each_entry_safe.
> >> >>
> >> >> This was done with Coccinelle.
> >> >>
> >> >> @@
> >> >> expression E1;
> >> >> identifier I1, I2;
> >> >> type T;
> >> >> iterator name list_for_each_entry_safe;
> >> >> @@
> >> >>
> >> >> T *I1;
> >> >> + T *tmp;
> >> >> ...
> >> >> - while (list_empty(&E1) == 0)
> >> >> + list_for_each_entry_safe (I1, tmp, &E1, I2)
> >> >> {
> >> >> ...when != T *I1;
> >> >> - I1 = list_entry(E1.next, T, I2);
> >> >> ...
> >> >> }
> >> >>
> >> >> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> >> >> ---
> >> >> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> >> >> 1 file changed, 4 insertions(+), 7 deletions(-)
> >> >>
> >> >> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> >> >> index ed8a0dc..e8b974f 100644
> >> >> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> >> >> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> >> >> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> >> struct cl_object *clobj = NULL;
> >> >> struct cl_page **pvec;
> >> >> struct osc_page *opg;
> >> >> + struct osc_page *tmp;
> >> >> int maxscan = 0;
> >> >> long count = 0;
> >> >> int index = 0;
> >> >> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> >> if (force)
> >> >> cli->cl_lru_reclaim++;
> >> >> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> >> >> - while (!list_empty(&cli->cl_lru_list)) {
> >> >> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> >> >> struct cl_page *page;
> >> >> bool will_free = false;
> >> >>
> >> >> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> >> >> if (--maxscan < 0)
> >> >> break;
> >> >>
> >> >> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> >> >> - ops_lru);
> >> >> page = opg->ops_cl.cpl_page;
> >> >> if (lru_page_busy(cli, page)) {
> >> >> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> >> >> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> >> >> {
> >> >> struct client_obd *stop_anchor = NULL;
> >> >> struct client_obd *cli;
> >> >> + struct client_obd *tmp;
> >> >> struct lu_env *env;
> >> >> long shrank = 0;
> >> >> u16 refcheck;
> >> >> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> >> >> return SHRINK_STOP;
> >> >>
> >> >> spin_lock(&osc_shrink_lock);
> >> >> - while (!list_empty(&osc_shrink_list)) {
> >> >> - cli = list_entry(osc_shrink_list.next, struct client_obd,
> >> >> - cl_shrink_list);
> >> >> -
> >> >> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> >> >> if (!stop_anchor)
> >> >> stop_anchor = cli;
> >> >> else if (cli == stop_anchor)
> >> >> --
> >> >> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> >> >> For more options, visit https://groups.google.com/d/optout.
> >> >>
> >>
>
[toc] | [prev] | [next] | [standalone]
| From | SIMRAN SINGHAL <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-05 18:50 +0100 |
| Subject | Re: [Outreachy kernel] [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <thIs9-3HM-13@gated-at.bofh.it> |
| In reply to | #1592804 |
On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> By the way, the above subject line is not correct. Normally one does not
> put .c/.h in the subject line. Indeed, there is not really any
> deterministic algorithm for choosing the subject line. You need to run
> git log --oneline filename to see what others have done.
So this would be fine Subject:-
staging: lustre: osc_page: Use list_for_each_entry_safe
>
> julia
>
> On Sun, 5 Mar 2017, simran singhal wrote:
>
>> Doubly linked lists which are iterated using list_empty
>> and list_entry macros have been replaced with list_for_each_entry_safe
>> macro.
>> This makes the iteration simpler and more readable.
>>
>> This patch replaces the while loop containing list_empty and list_entry
>> with list_for_each_entry_safe.
>>
>> This was done with Coccinelle.
>>
>> @@
>> expression E1;
>> identifier I1, I2;
>> type T;
>> iterator name list_for_each_entry_safe;
>> @@
>>
>> T *I1;
>> + T *tmp;
>> ...
>> - while (list_empty(&E1) == 0)
>> + list_for_each_entry_safe (I1, tmp, &E1, I2)
>> {
>> ...when != T *I1;
>> - I1 = list_entry(E1.next, T, I2);
>> ...
>> }
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>> ---
>> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
>> index ed8a0dc..e8b974f 100644
>> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
>> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
>> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> struct cl_object *clobj = NULL;
>> struct cl_page **pvec;
>> struct osc_page *opg;
>> + struct osc_page *tmp;
>> int maxscan = 0;
>> long count = 0;
>> int index = 0;
>> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (force)
>> cli->cl_lru_reclaim++;
>> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
>> - while (!list_empty(&cli->cl_lru_list)) {
>> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
>> struct cl_page *page;
>> bool will_free = false;
>>
>> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (--maxscan < 0)
>> break;
>>
>> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
>> - ops_lru);
>> page = opg->ops_cl.cpl_page;
>> if (lru_page_busy(cli, page)) {
>> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
>> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> {
>> struct client_obd *stop_anchor = NULL;
>> struct client_obd *cli;
>> + struct client_obd *tmp;
>> struct lu_env *env;
>> long shrank = 0;
>> u16 refcheck;
>> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> return SHRINK_STOP;
>>
>> spin_lock(&osc_shrink_lock);
>> - while (!list_empty(&osc_shrink_list)) {
>> - cli = list_entry(osc_shrink_list.next, struct client_obd,
>> - cl_shrink_list);
>> -
>> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
>> if (!stop_anchor)
>> stop_anchor = cli;
>> else if (cli == stop_anchor)
>> --
>> 2.7.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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
[toc] | [prev] | [next] | [standalone]
| From | James Simmons <jsimmons@infradead.org> |
|---|---|
| Date | 2017-03-06 16:30 +0100 |
| Subject | Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <ti2Ke-1BJ-11@gated-at.bofh.it> |
| In reply to | #1592799 |
> Doubly linked lists which are iterated using list_empty
> and list_entry macros have been replaced with list_for_each_entry_safe
> macro.
> This makes the iteration simpler and more readable.
>
> This patch replaces the while loop containing list_empty and list_entry
> with list_for_each_entry_safe.
>
> This was done with Coccinelle.
>
> @@
> expression E1;
> identifier I1, I2;
> type T;
> iterator name list_for_each_entry_safe;
> @@
>
> T *I1;
> + T *tmp;
> ...
> - while (list_empty(&E1) == 0)
> + list_for_each_entry_safe (I1, tmp, &E1, I2)
> {
> ...when != T *I1;
> - I1 = list_entry(E1.next, T, I2);
> ...
> }
>
> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
NAK!!!!!!
This change was reverted in commit
cd15dd6ef4ea11df87f717b8b1b83aaa738ec8af
Doing these while (list_empty(..)) to list_for_entry...
are not simple changes and have broken things in lustre
before. Unless you really understand the state machine of
the lustre code I don't recommend these kinds of change
for lustre.
> ---
> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
> index ed8a0dc..e8b974f 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> struct cl_object *clobj = NULL;
> struct cl_page **pvec;
> struct osc_page *opg;
> + struct osc_page *tmp;
> int maxscan = 0;
> long count = 0;
> int index = 0;
> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (force)
> cli->cl_lru_reclaim++;
> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
> - while (!list_empty(&cli->cl_lru_list)) {
> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
> struct cl_page *page;
> bool will_free = false;
>
> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
> if (--maxscan < 0)
> break;
>
> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
> - ops_lru);
> page = opg->ops_cl.cpl_page;
> if (lru_page_busy(cli, page)) {
> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> {
> struct client_obd *stop_anchor = NULL;
> struct client_obd *cli;
> + struct client_obd *tmp;
> struct lu_env *env;
> long shrank = 0;
> u16 refcheck;
> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
> return SHRINK_STOP;
>
> spin_lock(&osc_shrink_lock);
> - while (!list_empty(&osc_shrink_list)) {
> - cli = list_entry(osc_shrink_list.next, struct client_obd,
> - cl_shrink_list);
> -
> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
> if (!stop_anchor)
> stop_anchor = cli;
> else if (cli == stop_anchor)
> --
> 2.7.4
>
>
[toc] | [prev] | [next] | [standalone]
| From | SIMRAN SINGHAL <singhalsimran0@gmail.com> |
|---|---|
| Date | 2017-03-06 20:30 +0100 |
| Subject | Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <ti6ut-4kC-7@gated-at.bofh.it> |
| In reply to | #1593471 |
On Mon, Mar 6, 2017 at 8:50 PM, James Simmons <jsimmons@infradead.org> wrote:
>
>> Doubly linked lists which are iterated using list_empty
>> and list_entry macros have been replaced with list_for_each_entry_safe
>> macro.
>> This makes the iteration simpler and more readable.
>>
>> This patch replaces the while loop containing list_empty and list_entry
>> with list_for_each_entry_safe.
>>
>> This was done with Coccinelle.
>>
>> @@
>> expression E1;
>> identifier I1, I2;
>> type T;
>> iterator name list_for_each_entry_safe;
>> @@
>>
>> T *I1;
>> + T *tmp;
>> ...
>> - while (list_empty(&E1) == 0)
>> + list_for_each_entry_safe (I1, tmp, &E1, I2)
>> {
>> ...when != T *I1;
>> - I1 = list_entry(E1.next, T, I2);
>> ...
>> }
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>
> NAK!!!!!!
>
> This change was reverted in commit
>
> cd15dd6ef4ea11df87f717b8b1b83aaa738ec8af
>
> Doing these while (list_empty(..)) to list_for_entry...
> are not simple changes and have broken things in lustre
> before. Unless you really understand the state machine of
> the lustre code I don't recommend these kinds of change
> for lustre.
>
Is this for this particular patch or for all the patches of this patch-series.
>> ---
>> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
>> index ed8a0dc..e8b974f 100644
>> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
>> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
>> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> struct cl_object *clobj = NULL;
>> struct cl_page **pvec;
>> struct osc_page *opg;
>> + struct osc_page *tmp;
>> int maxscan = 0;
>> long count = 0;
>> int index = 0;
>> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (force)
>> cli->cl_lru_reclaim++;
>> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
>> - while (!list_empty(&cli->cl_lru_list)) {
>> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
>> struct cl_page *page;
>> bool will_free = false;
>>
>> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (--maxscan < 0)
>> break;
>>
>> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
>> - ops_lru);
>> page = opg->ops_cl.cpl_page;
>> if (lru_page_busy(cli, page)) {
>> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
>> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> {
>> struct client_obd *stop_anchor = NULL;
>> struct client_obd *cli;
>> + struct client_obd *tmp;
>> struct lu_env *env;
>> long shrank = 0;
>> u16 refcheck;
>> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> return SHRINK_STOP;
>>
>> spin_lock(&osc_shrink_lock);
>> - while (!list_empty(&osc_shrink_list)) {
>> - cli = list_entry(osc_shrink_list.next, struct client_obd,
>> - cl_shrink_list);
>> -
>> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
>> if (!stop_anchor)
>> stop_anchor = cli;
>> else if (cli == stop_anchor)
>> --
>> 2.7.4
>>
>>
[toc] | [prev] | [next] | [standalone]
| From | "Dilger, Andreas" <andreas.dilger@intel.com> |
|---|---|
| Date | 2017-03-07 13:10 +0100 |
| Subject | Re: [PATCH 5/5] staging: lustre: osc_page.c: Use list_for_each_entry_safe |
| Message-ID | <tim6e-7io-15@gated-at.bofh.it> |
| In reply to | #1593471 |
On Mar 6, 2017, at 08:20, James Simmons <jsimmons@infradead.org> wrote:
>
>>
>> Doubly linked lists which are iterated using list_empty
>> and list_entry macros have been replaced with list_for_each_entry_safe
>> macro.
>> This makes the iteration simpler and more readable.
>>
>> This patch replaces the while loop containing list_empty and list_entry
>> with list_for_each_entry_safe.
>>
>> This was done with Coccinelle.
>>
>> @@
>> expression E1;
>> identifier I1, I2;
>> type T;
>> iterator name list_for_each_entry_safe;
>> @@
>>
>> T *I1;
>> + T *tmp;
>> ...
>> - while (list_empty(&E1) == 0)
>> + list_for_each_entry_safe (I1, tmp, &E1, I2)
>> {
>> ...when != T *I1;
>> - I1 = list_entry(E1.next, T, I2);
>> ...
>> }
>>
>> Signed-off-by: simran singhal <singhalsimran0@gmail.com>
>
> NAK!!!!!!
>
> This change was reverted in commit
>
> cd15dd6ef4ea11df87f717b8b1b83aaa738ec8af
>
> Doing these while (list_empty(..)) to list_for_entry...
> are not simple changes and have broken things in lustre
> before. Unless you really understand the state machine of
> the lustre code I don't recommend these kinds of change
> for lustre.
It may be useful to add a comment to these cases where the while() loop cannot be
replaced by list_for_each_entry_safe() (with details of why that is the case) to
avoid such optimization attempts again in the future.
Cheers, Andreas
>
>> ---
>> drivers/staging/lustre/lustre/osc/osc_page.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/osc/osc_page.c b/drivers/staging/lustre/lustre/osc/osc_page.c
>> index ed8a0dc..e8b974f 100644
>> --- a/drivers/staging/lustre/lustre/osc/osc_page.c
>> +++ b/drivers/staging/lustre/lustre/osc/osc_page.c
>> @@ -542,6 +542,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> struct cl_object *clobj = NULL;
>> struct cl_page **pvec;
>> struct osc_page *opg;
>> + struct osc_page *tmp;
>> int maxscan = 0;
>> long count = 0;
>> int index = 0;
>> @@ -572,7 +573,7 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (force)
>> cli->cl_lru_reclaim++;
>> maxscan = min(target << 1, atomic_long_read(&cli->cl_lru_in_list));
>> - while (!list_empty(&cli->cl_lru_list)) {
>> + list_for_each_entry_safe(opg, tmp, &cli->cl_lru_list, ops_lru) {
>> struct cl_page *page;
>> bool will_free = false;
>>
>> @@ -582,8 +583,6 @@ long osc_lru_shrink(const struct lu_env *env, struct client_obd *cli,
>> if (--maxscan < 0)
>> break;
>>
>> - opg = list_entry(cli->cl_lru_list.next, struct osc_page,
>> - ops_lru);
>> page = opg->ops_cl.cpl_page;
>> if (lru_page_busy(cli, page)) {
>> list_move_tail(&opg->ops_lru, &cli->cl_lru_list);
>> @@ -1043,6 +1042,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> {
>> struct client_obd *stop_anchor = NULL;
>> struct client_obd *cli;
>> + struct client_obd *tmp;
>> struct lu_env *env;
>> long shrank = 0;
>> u16 refcheck;
>> @@ -1059,10 +1059,7 @@ unsigned long osc_cache_shrink_scan(struct shrinker *sk,
>> return SHRINK_STOP;
>>
>> spin_lock(&osc_shrink_lock);
>> - while (!list_empty(&osc_shrink_list)) {
>> - cli = list_entry(osc_shrink_list.next, struct client_obd,
>> - cl_shrink_list);
>> -
>> + list_for_each_entry_safe(cli, tmp, &osc_shrink_list, cl_shrink_list) {
>> if (!stop_anchor)
>> stop_anchor = cli;
>> else if (cli == stop_anchor)
>> --
>> 2.7.4
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web