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
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 <[email protected]>
---
drivers/staging/lustre/lustre/ptlrpc/sec_gc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
index 8ffd000..fe1c0af 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
@@ -98,12 +98,11 @@ void sptlrpc_gc_del_sec(struct ptlrpc_sec *sec)
static void sec_process_ctx_list(void)
{
struct ptlrpc_cli_ctx *ctx;
+ struct ptlrpc_cli_ctx *tmp;
spin_lock(&sec_gc_ctx_list_lock);
- while (!list_empty(&sec_gc_ctx_list)) {
- ctx = list_entry(sec_gc_ctx_list.next,
- struct ptlrpc_cli_ctx, cc_gc_chain);
+ list_for_each_entry_safe(ctx, tmp, &sec_gc_ctx_list, cc_gc_chain) {
list_del_init(&ctx->cc_gc_chain);
spin_unlock(&sec_gc_ctx_list_lock);
--
2.7.4
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 <[email protected]>
---
drivers/staging/lustre/lustre/osc/osc_cache.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index f8c5fc0..caa5fec 100644
--- a/drivers/staging/lustre/lustre/osc/osc_cache.c
+++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
@@ -1996,6 +1996,7 @@ static unsigned int get_write_extents(struct osc_object *obj,
{
struct client_obd *cli = osc_cli(obj);
struct osc_extent *ext;
+ struct osc_extent *tmp;
struct osc_extent *temp;
struct extent_rpc_data data = {
.erd_rpc_list = rpclist,
@@ -2014,9 +2015,7 @@ static unsigned int get_write_extents(struct osc_object *obj,
if (data.erd_page_count == data.erd_max_pages)
return data.erd_page_count;
- while (!list_empty(&obj->oo_urgent_exts)) {
- ext = list_entry(obj->oo_urgent_exts.next,
- struct osc_extent, oe_link);
+ list_for_each_entry_safe(ext, tmp, &obj->oo_urgent_exts, oe_link) {
if (!try_to_add_extent_for_io(cli, ext, &data))
return data.erd_page_count;
--
2.7.4
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 <[email protected]>
---
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
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 <[email protected]>
---
drivers/staging/lustre/lustre/ptlrpc/service.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
index b8091c1..4e7dc1d 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/service.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
@@ -2314,6 +2314,7 @@ static void ptlrpc_svcpt_stop_threads(struct ptlrpc_service_part *svcpt)
{
struct l_wait_info lwi = { 0 };
struct ptlrpc_thread *thread;
+ struct ptlrpc_thread *tmp;
LIST_HEAD(zombie);
CDEBUG(D_INFO, "Stopping threads for service %s\n",
@@ -2329,9 +2330,7 @@ static void ptlrpc_svcpt_stop_threads(struct ptlrpc_service_part *svcpt)
wake_up_all(&svcpt->scp_waitq);
- while (!list_empty(&svcpt->scp_threads)) {
- thread = list_entry(svcpt->scp_threads.next,
- struct ptlrpc_thread, t_link);
+ list_for_each_entry_safe(thread, tmp, &svcpt->scp_threads, t_link) {
if (thread_is_stopped(thread)) {
list_del(&thread->t_link);
list_add(&thread->t_link, &zombie);
--
2.7.4
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
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 <[email protected]>
> ---
> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
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 <[email protected]>
> > ---
> > 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
On Sun, Mar 5, 2017 at 11:09 PM, Julia Lawall <[email protected]> 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.
>>
>> 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 <[email protected]>
>> ---
>> drivers/staging/lustre/lustre/ptlrpc/sec_gc.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
>> index 8ffd000..fe1c0af 100644
>> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
>> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
>> @@ -98,12 +98,11 @@ void sptlrpc_gc_del_sec(struct ptlrpc_sec *sec)
>> static void sec_process_ctx_list(void)
>> {
>> struct ptlrpc_cli_ctx *ctx;
>> + struct ptlrpc_cli_ctx *tmp;
>
> Another improvement would be to define both variables at once:
>
> T *I1
> + , *tmp
> ;
>
This is particulary for this patch or for all the patches of this patch-series.
> julia
>
>>
>> spin_lock(&sec_gc_ctx_list_lock);
>>
>> - while (!list_empty(&sec_gc_ctx_list)) {
>> - ctx = list_entry(sec_gc_ctx_list.next,
>> - struct ptlrpc_cli_ctx, cc_gc_chain);
>> + list_for_each_entry_safe(ctx, tmp, &sec_gc_ctx_list, cc_gc_chain) {
>> list_del_init(&ctx->cc_gc_chain);
>> spin_unlock(&sec_gc_ctx_list_lock);
>>
>> --
>> 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 [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/1488733610-22289-3-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
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 <[email protected]>
> ---
> drivers/staging/lustre/lustre/ptlrpc/sec_gc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> index 8ffd000..fe1c0af 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> @@ -98,12 +98,11 @@ void sptlrpc_gc_del_sec(struct ptlrpc_sec *sec)
> static void sec_process_ctx_list(void)
> {
> struct ptlrpc_cli_ctx *ctx;
> + struct ptlrpc_cli_ctx *tmp;
Another improvement would be to define both variables at once:
T *I1
+ , *tmp
;
julia
>
> spin_lock(&sec_gc_ctx_list_lock);
>
> - while (!list_empty(&sec_gc_ctx_list)) {
> - ctx = list_entry(sec_gc_ctx_list.next,
> - struct ptlrpc_cli_ctx, cc_gc_chain);
> + list_for_each_entry_safe(ctx, tmp, &sec_gc_ctx_list, cc_gc_chain) {
> list_del_init(&ctx->cc_gc_chain);
> spin_unlock(&sec_gc_ctx_list_lock);
>
> --
> 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 [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/1488733610-22289-3-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> On Sun, Mar 5, 2017 at 11:09 PM, Julia Lawall <[email protected]> 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.
> >>
> >> 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 <[email protected]>
> >> ---
> >> drivers/staging/lustre/lustre/ptlrpc/sec_gc.c | 5 ++---
> >> 1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> >> index 8ffd000..fe1c0af 100644
> >> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> >> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_gc.c
> >> @@ -98,12 +98,11 @@ void sptlrpc_gc_del_sec(struct ptlrpc_sec *sec)
> >> static void sec_process_ctx_list(void)
> >> {
> >> struct ptlrpc_cli_ctx *ctx;
> >> + struct ptlrpc_cli_ctx *tmp;
> >
> > Another improvement would be to define both variables at once:
> >
> > T *I1
> > + , *tmp
> > ;
> >
> This is particulary for this patch or for all the patches of this patch-series.
All, I would guess. Unless the line gets too long.
julia
>
> > julia
> >
> >>
> >> spin_lock(&sec_gc_ctx_list_lock);
> >>
> >> - while (!list_empty(&sec_gc_ctx_list)) {
> >> - ctx = list_entry(sec_gc_ctx_list.next,
> >> - struct ptlrpc_cli_ctx, cc_gc_chain);
> >> + list_for_each_entry_safe(ctx, tmp, &sec_gc_ctx_list, cc_gc_chain) {
> >> list_del_init(&ctx->cc_gc_chain);
> >> spin_unlock(&sec_gc_ctx_list_lock);
> >>
> >> --
> >> 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 [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/1488733610-22289-3-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <[email protected]> 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 <[email protected]>
>> ---
>> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
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;
> ...
The function that is modified in this patch actually has another
opportunity. That doesn't get transformed because with ... Coccinelle
matches the shortest path between what comes before and what comes after,
and so it only matches the first while loop.
You could sort of fix the problem by putting when any on the ... That
allows anything in the path, including other while loops.
That though will mean that you will try to add two instances of the tmp
declaration after the declaration of I1. Coccinelle allows adding only
one thing, unless the + is replaced by ++. But if you do that, you will
get two decarations of tmp.
So either make the second change by hand, or let Coccinelle do it an
remove the second declaration of tmp by hand.
julia
> - 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 <[email protected]>
> ---
> drivers/staging/lustre/lustre/ptlrpc/service.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
> index b8091c1..4e7dc1d 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/service.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
> @@ -2314,6 +2314,7 @@ static void ptlrpc_svcpt_stop_threads(struct ptlrpc_service_part *svcpt)
> {
> struct l_wait_info lwi = { 0 };
> struct ptlrpc_thread *thread;
> + struct ptlrpc_thread *tmp;
> LIST_HEAD(zombie);
>
> CDEBUG(D_INFO, "Stopping threads for service %s\n",
> @@ -2329,9 +2330,7 @@ static void ptlrpc_svcpt_stop_threads(struct ptlrpc_service_part *svcpt)
>
> wake_up_all(&svcpt->scp_waitq);
>
> - while (!list_empty(&svcpt->scp_threads)) {
> - thread = list_entry(svcpt->scp_threads.next,
> - struct ptlrpc_thread, t_link);
> + list_for_each_entry_safe(thread, tmp, &svcpt->scp_threads, t_link) {
> if (thread_is_stopped(thread)) {
> list_del(&thread->t_link);
> list_add(&thread->t_link, &zombie);
> --
> 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 [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/1488733610-22289-2-git-send-email-singhalsimran0%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> On Sun, Mar 5, 2017 at 11:18 PM, Julia Lawall <[email protected]> wrote:
> >
> >
> > On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
> >
> >> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <[email protected]> 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 <[email protected]>
> >> >> ---
> >> >> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
> >> >> For more options, visit https://groups.google.com/d/optout.
> >> >>
> >>
>
On Sun, Mar 5, 2017 at 11:18 PM, Julia Lawall <[email protected]> wrote:
>
>
> On Sun, 5 Mar 2017, SIMRAN SINGHAL wrote:
>
>> On Sun, Mar 5, 2017 at 10:47 PM, Julia Lawall <[email protected]> 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 <[email protected]>
>> >> ---
>> >> 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 [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/1488733610-22289-6-git-send-email-singhalsimran0%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
> 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 <[email protected]>
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
>
>
On Mon, Mar 6, 2017 at 8:50 PM, James Simmons <[email protected]> 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 <[email protected]>
>
> 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
>>
>>
On Mar 6, 2017, at 08:20, James Simmons <[email protected]> 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 <[email protected]>
>
> 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