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.
>
>