* [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
@ 2016-09-15 19:43 Sandhya Bankar
2016-09-15 20:05 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Sandhya Bankar @ 2016-09-15 19:43 UTC (permalink / raw)
To: outreachy-kernel
Cc: oleg.drokin, andreas.dilger, jsimmons, gregkh, jinshan.xiong,
lai.siyao, bobijam.xu, mike.rapoport
Convert array index from the loop bound to the loop index.
Used below semantic patch to identify this issue:
@@
expression e1,e2,ar;
@@
for(e1 = 0; e1 < e2; e1++) { <...
ar[
- e2
+ e1
]
...> }
Signed-off-by: Sandhya Bankar <bankarsandhya512@gmail.com>
---
drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index 91a5806..eeccd30 100755
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
* thus we must uninitialize up to i, the rest are undefined.
*/
for (j = 0; j < i; j++) {
- cle = &cl_env_percpu[i];
+ cle = &cl_env_percpu[j];
lu_context_exit(&cle->ce_ses);
lu_context_fini(&cle->ce_ses);
lu_env_fini(&cle->ce_lu);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
2016-09-15 19:43 [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index Sandhya Bankar
@ 2016-09-15 20:05 ` Julia Lawall
2016-09-15 20:53 ` sandhya bankar
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-09-15 20:05 UTC (permalink / raw)
To: Sandhya Bankar
Cc: outreachy-kernel, oleg.drokin, andreas.dilger, jsimmons, gregkh,
jinshan.xiong, lai.siyao, bobijam.xu, mike.rapoport
On Fri, 16 Sep 2016, Sandhya Bankar wrote:
> Convert array index from the loop bound to the loop index.
> Used below semantic patch to identify this issue:
This is a really nice idea! However, it would be good for the commit
message to explain why the change is correct. Certainly the code is
suspicious, but why is it actually wrong? The idea would be to consider
what is done with cle on each iteration.
julia
> @@
> expression e1,e2,ar;
> @@
>
> for(e1 = 0; e1 < e2; e1++) { <...
> ar[
> - e2
> + e1
> ]
> ...> }
>
>
> Signed-off-by: Sandhya Bankar <bankarsandhya512@gmail.com>
> ---
> drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index 91a5806..eeccd30 100755
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
> * thus we must uninitialize up to i, the rest are undefined.
> */
> for (j = 0; j < i; j++) {
> - cle = &cl_env_percpu[i];
> + cle = &cl_env_percpu[j];
> lu_context_exit(&cle->ce_ses);
> lu_context_fini(&cle->ce_ses);
> lu_env_fini(&cle->ce_lu);
> --
> 1.7.1
>
> --
> 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/20160915194341.GA30843%40sandhya.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
2016-09-15 20:05 ` [Outreachy kernel] " Julia Lawall
@ 2016-09-15 20:53 ` sandhya bankar
[not found] ` <706CE650-AC74-4C04-97F7-54A6A03E6C27@intel.com>
0 siblings, 1 reply; 6+ messages in thread
From: sandhya bankar @ 2016-09-15 20:53 UTC (permalink / raw)
To: Julia Lawall
Cc: outreachy-kernel, oleg.drokin, andreas.dilger, jsimmons, Greg KH,
jinshan.xiong, lai.siyao, bobijam.xu, mike.rapoport
[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]
The structure cl_env_percpu[NR_CPUS] have been initializing for each
possible cpu (i.e 0 to i) but during
initialization if any error will occurred this come to the error handling
code in which the cl_env_percpu should be uninitialized upto i but
currently uninitialization happening for the cl_env_percpu[i] which does
not seems to be correct. That is why proposing this fix. Please Correct me
,If I am wrong. Should I send patch v2 with this issue description ?
On Fri, Sep 16, 2016 at 1:35 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Fri, 16 Sep 2016, Sandhya Bankar wrote:
>
> > Convert array index from the loop bound to the loop index.
> > Used below semantic patch to identify this issue:
>
> This is a really nice idea! However, it would be good for the commit
> message to explain why the change is correct. Certainly the code is
> suspicious, but why is it actually wrong? The idea would be to consider
> what is done with cle on each iteration.
>
> julia
>
> > @@
> > expression e1,e2,ar;
> > @@
> >
> > for(e1 = 0; e1 < e2; e1++) { <...
> > ar[
> > - e2
> > + e1
> > ]
> > ...> }
> >
> >
> > Signed-off-by: Sandhya Bankar <bankarsandhya512@gmail.com>
> > ---
> > drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > index 91a5806..eeccd30 100755
> > --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > @@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
> > * thus we must uninitialize up to i, the rest are
> undefined.
> > */
> > for (j = 0; j < i; j++) {
> > - cle = &cl_env_percpu[i];
> > + cle = &cl_env_percpu[j];
> > lu_context_exit(&cle->ce_ses);
> > lu_context_fini(&cle->ce_ses);
> > lu_env_fini(&cle->ce_lu);
> > --
> > 1.7.1
> >
> > --
> > 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/20160915194341.GA30843%40sandhya.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
[-- Attachment #2: Type: text/html, Size: 3927 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
[not found] ` <706CE650-AC74-4C04-97F7-54A6A03E6C27@intel.com>
@ 2016-09-16 5:47 ` Julia Lawall
2016-09-16 8:35 ` sandhya bankar
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-09-16 5:47 UTC (permalink / raw)
To: Dilger, Andreas
Cc: bankarsandhya512, Julia Lawall, outreachy-kernel, Drokin, Oleg,
James Simmons, Greg KH, Xiong, Jinshan, Siyao, Lai, Xu, Bobijam,
mike.rapoport
On Thu, 15 Sep 2016, Dilger, Andreas wrote:
>
> > On Sep 15, 2016, at 14:53, sandhya bankar <bankarsandhya512@gmail.com> wrote:
> >
> > The structure cl_env_percpu[NR_CPUS] have been initializing for each
> > possible cpu (i.e 0 to i) but during initialization if any error will
> > occurred this come to the error handling code in which the
> > cl_env_percpu should be uninitialized upto i but currently
Sandhya,
"should be uninitialized" is a bit unclear, because it could describe the
state that should hold, or the state that you should achieve. It could be
better to say "the error handling code which should
uninitialize/free/release (whatever seems best) the cl_env_percpu ...".
Andreas's suggestion is also good. It says that same thing in another
way, and thus makes everything more clear.
Congratulations on finding a real bug :)
julia
> > uninitialization happening for the cl_env_percpu[i] which does not
> > seems to be correct. That is why proposing this fix. Please Correct me
> > ,If I am wrong. Should I send patch v2 with this issue description ?
>
> This is definitely a bug, though thankfully only in an error handling path.
>
> Your description is much more clear. You might want to write
>
> "... but currently cleanup is happening repeatedly for the same
> cl_env_percpu[i] element."
>
> Cheers, Andreas
>
> > On Fri, Sep 16, 2016 at 1:35 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >>
> >>
> >> On Fri, 16 Sep 2016, Sandhya Bankar wrote:
> >>
> >> > Convert array index from the loop bound to the loop index.
> >> > Used below semantic patch to identify this issue:
> >>
> >> This is a really nice idea! However, it would be good for the commit
> >> message to explain why the change is correct. Certainly the code is
> >> suspicious, but why is it actually wrong? The idea would be to consider
> >> what is done with cle on each iteration.
> >>
> >> julia
> >>
> >> > @@
> >> > expression e1,e2,ar;
> >> > @@
> >> >
> >> > for(e1 = 0; e1 < e2; e1++) { <...
> >> > ar[
> >> > - e2
> >> > + e1
> >> > ]
> >> > ...> }
> >> >
> >> >
> >> > Signed-off-by: Sandhya Bankar <bankarsandhya512@gmail.com>
> >> > ---
> >> > drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
> >> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >> >
> >> > diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> > index 91a5806..eeccd30 100755
> >> > --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> > @@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
> >> > * thus we must uninitialize up to i, the rest are undefined.
> >> > */
> >> > for (j = 0; j < i; j++) {
> >> > - cle = &cl_env_percpu[i];
> >> > + cle = &cl_env_percpu[j];
> >> > lu_context_exit(&cle->ce_ses);
> >> > lu_context_fini(&cle->ce_ses);
> >> > lu_env_fini(&cle->ce_lu);
> >> > --
> >> > 1.7.1
> >> >
> >> > --
> >> > 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/20160915194341.GA30843%40sandhya.
> >> > For more options, visit https://groups.google.com/d/optout.
> >> >
> >>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
2016-09-16 5:47 ` Julia Lawall
@ 2016-09-16 8:35 ` sandhya bankar
2016-09-16 8:38 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: sandhya bankar @ 2016-09-16 8:35 UTC (permalink / raw)
To: Julia Lawall
Cc: Dilger, Andreas, outreachy-kernel, Drokin, Oleg, James Simmons,
Greg KH, Xiong, Jinshan, Siyao, Lai, Xu, Bobijam, mike.rapoport
[-- Attachment #1: Type: text/plain, Size: 4035 bytes --]
Thanks Julia and Andreas.
Should I need to send patch v2 as per your suggestions ?
On Fri, Sep 16, 2016 at 11:17 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Thu, 15 Sep 2016, Dilger, Andreas wrote:
>
> >
> > > On Sep 15, 2016, at 14:53, sandhya bankar <bankarsandhya512@gmail.com>
> wrote:
> > >
> > > The structure cl_env_percpu[NR_CPUS] have been initializing for each
> > > possible cpu (i.e 0 to i) but during initialization if any error will
> > > occurred this come to the error handling code in which the
> > > cl_env_percpu should be uninitialized upto i but currently
>
> Sandhya,
>
> "should be uninitialized" is a bit unclear, because it could describe the
> state that should hold, or the state that you should achieve. It could be
> better to say "the error handling code which should
> uninitialize/free/release (whatever seems best) the cl_env_percpu ...".
>
> Andreas's suggestion is also good. It says that same thing in another
> way, and thus makes everything more clear.
>
> Congratulations on finding a real bug :)
>
> julia
>
> > > uninitialization happening for the cl_env_percpu[i] which does not
> > > seems to be correct. That is why proposing this fix. Please Correct me
> > > ,If I am wrong. Should I send patch v2 with this issue description ?
> >
> > This is definitely a bug, though thankfully only in an error handling
> path.
> >
> > Your description is much more clear. You might want to write
> >
> > "... but currently cleanup is happening repeatedly for the same
> > cl_env_percpu[i] element."
> >
> > Cheers, Andreas
> >
> > > On Fri, Sep 16, 2016 at 1:35 AM, Julia Lawall <julia.lawall@lip6.fr>
> wrote:
> > >>
> > >>
> > >> On Fri, 16 Sep 2016, Sandhya Bankar wrote:
> > >>
> > >> > Convert array index from the loop bound to the loop index.
> > >> > Used below semantic patch to identify this issue:
> > >>
> > >> This is a really nice idea! However, it would be good for the commit
> > >> message to explain why the change is correct. Certainly the code is
> > >> suspicious, but why is it actually wrong? The idea would be to
> consider
> > >> what is done with cle on each iteration.
> > >>
> > >> julia
> > >>
> > >> > @@
> > >> > expression e1,e2,ar;
> > >> > @@
> > >> >
> > >> > for(e1 = 0; e1 < e2; e1++) { <...
> > >> > ar[
> > >> > - e2
> > >> > + e1
> > >> > ]
> > >> > ...> }
> > >> >
> > >> >
> > >> > Signed-off-by: Sandhya Bankar <bankarsandhya512@gmail.com>
> > >> > ---
> > >> > drivers/staging/lustre/lustre/obdclass/cl_object.c | 2 +-
> > >> > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >> >
> > >> > diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > index 91a5806..eeccd30 100755
> > >> > --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > @@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
> > >> > * thus we must uninitialize up to i, the rest are
> undefined.
> > >> > */
> > >> > for (j = 0; j < i; j++) {
> > >> > - cle = &cl_env_percpu[i];
> > >> > + cle = &cl_env_percpu[j];
> > >> > lu_context_exit(&cle->ce_ses);
> > >> > lu_context_fini(&cle->ce_ses);
> > >> > lu_env_fini(&cle->ce_lu);
> > >> > --
> > >> > 1.7.1
> > >> >
> > >> > --
> > >> > 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/20160915194341.GA30843%
> 40sandhya.
> > >> > For more options, visit https://groups.google.com/d/optout.
> > >> >
> > >>
> >
> >
>
[-- Attachment #2: Type: text/html, Size: 6191 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index.
2016-09-16 8:35 ` sandhya bankar
@ 2016-09-16 8:38 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2016-09-16 8:38 UTC (permalink / raw)
To: sandhya bankar
Cc: Julia Lawall, Dilger, Andreas, outreachy-kernel, Drokin, Oleg,
James Simmons, Greg KH, Xiong, Jinshan, Siyao, Lai, Xu, Bobijam,
mike.rapoport
[-- Attachment #1: Type: TEXT/PLAIN, Size: 5488 bytes --]
On Fri, 16 Sep 2016, sandhya bankar wrote:
> Thanks Julia and Andreas.
> Should I need to send patch v2 as per your suggestions ?
Yes.
julia
>
> On Fri, Sep 16, 2016 at 11:17 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Thu, 15 Sep 2016, Dilger, Andreas wrote:
>
> >
> > > On Sep 15, 2016, at 14:53, sandhya bankar
> <bankarsandhya512@gmail.com> wrote:
> > >
> > > The structure cl_env_percpu[NR_CPUS] have been initializing
> for each
> > > possible cpu (i.e 0 to i) but during initialization if any
> error will
> > > occurred this come to the error handling code in which the
> > > cl_env_percpu should be uninitialized upto i but currently
>
> Sandhya,
>
> "should be uninitialized" is a bit unclear, because it could
> describe the
> state that should hold, or the state that you should achieve.
> It could be
> better to say "the error handling code which should
> uninitialize/free/release (whatever seems best) the
> cl_env_percpu ...".
>
> Andreas's suggestion is also good. It says that same thing in
> another
> way, and thus makes everything more clear.
>
> Congratulations on finding a real bug :)
>
> julia
>
> > > uninitialization happening for the cl_env_percpu[i] which
> does not
> > > seems to be correct. That is why proposing this fix. Please
> Correct me
> > > ,If I am wrong. Should I send patch v2 with this issue
> description ?
> >
> > This is definitely a bug, though thankfully only in an error
> handling path.
> >
> > Your description is much more clear. You might want to write
> >
> > "... but currently cleanup is happening repeatedly for the
> same
> > cl_env_percpu[i] element."
> >
> > Cheers, Andreas
> >
> > > On Fri, Sep 16, 2016 at 1:35 AM, Julia Lawall
> <julia.lawall@lip6.fr> wrote:
> > >>
> > >>
> > >> On Fri, 16 Sep 2016, Sandhya Bankar wrote:
> > >>
> > >> > Convert array index from the loop bound to the loop
> index.
> > >> > Used below semantic patch to identify this issue:
> > >>
> > >> This is a really nice idea! However, it would be good for
> the commit
> > >> message to explain why the change is correct. Certainly
> the code is
> > >> suspicious, but why is it actually wrong? The idea would
> be to consider
> > >> what is done with cle on each iteration.
> > >>
> > >> julia
> > >>
> > >> > @@
> > >> > expression e1,e2,ar;
> > >> > @@
> > >> >
> > >> > for(e1 = 0; e1 < e2; e1++) { <...
> > >> > ar[
> > >> > - e2
> > >> > + e1
> > >> > ]
> > >> > ...> }
> > >> >
> > >> >
> > >> > Signed-off-by: Sandhya Bankar
> <bankarsandhya512@gmail.com>
> > >> > ---
> > >> > drivers/staging/lustre/lustre/obdclass/cl_object.c |
> 2 +-
> > >> > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >> >
> > >> > diff --git
> a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > index 91a5806..eeccd30 100755
> > >> > --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> > >> > @@ -1000,7 +1000,7 @@ static int cl_env_percpu_init(void)
> > >> > * thus we must uninitialize up to i, the
> rest are undefined.
> > >> > */
> > >> > for (j = 0; j < i; j++) {
> > >> > - cle = &cl_env_percpu[i];
> > >> > + cle = &cl_env_percpu[j];
> > >> > lu_context_exit(&cle->ce_ses);
> > >> > lu_context_fini(&cle->ce_ses);
> > >> > lu_env_fini(&cle->ce_lu);
> > >> > --
> > >> > 1.7.1
> > >> >
> > >> > --
> > >> > 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 visithttps://groups.google.com/d/msgid/outreachy-kernel/20160915194341.GA30843%4
> 0sandhya.
> > >> > For more options, visit
> https://groups.google.com/d/optout.
> > >> >
> > >>
> >
> >
>
>
> --
> 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 visithttps://groups.google.com/d/msgid/outreachy-kernel/CAA3ASz%3Dt4wZOkk%3DBj8i
> YLMFHwzCj%2BE%2B0BbC9UsV%2BL17oToh3ZA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-16 8:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 19:43 [PATCH] Staging: lustre: Convert array index from the loop bound to the loop index Sandhya Bankar
2016-09-15 20:05 ` [Outreachy kernel] " Julia Lawall
2016-09-15 20:53 ` sandhya bankar
[not found] ` <706CE650-AC74-4C04-97F7-54A6A03E6C27@intel.com>
2016-09-16 5:47 ` Julia Lawall
2016-09-16 8:35 ` sandhya bankar
2016-09-16 8:38 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.