lustre: question about lov_request.c

lustre: question about lov_request.c

[Julia Lawall]

Hello,

The function lov_finish_set in 
drivers/staging/lustre/lustre/lov/lov_request.c contains the code:

       if (set->set_pga) {
                int len = set->set_oabufs * sizeof(*set->set_pga);
                OBD_FREE_LARGE(set->set_pga, len);
        }

If I change the call to OBD_FREE_LARGE to kvfree, then len is not useful 
any more.  But actually, at least with grep, I can't find anywhere that 
either the set_pga field or the set_oabufs field is set.  Am I missing 
something, or can the whole if be removed?  Can these two fields go too?

thanks,
julia

Re: lustre: question about lov_request.c

[Drokin, Oleg]

Hello!

  You are right, set_pga seems to be a dead member. It was alive a once, but somehow not fully removed now,
  so it's safe to drop the whole if and also the struct member itself.
  set_oabufs could be dropped as well.

  Thanks.

Bye,
    Oleg
On Jun 7, 2015, at 4:11 PM, Julia Lawall wrote:

> Hello,
> 
> The function lov_finish_set in 
> drivers/staging/lustre/lustre/lov/lov_request.c contains the code:
> 
>       if (set->set_pga) {
>                int len = set->set_oabufs * sizeof(*set->set_pga);
>                OBD_FREE_LARGE(set->set_pga, len);
>        }
> 
> If I change the call to OBD_FREE_LARGE to kvfree, then len is not useful 
> any more.  But actually, at least with grep, I can't find anywhere that 
> either the set_pga field or the set_oabufs field is set.  Am I missing 
> something, or can the whole if be removed?  Can these two fields go too?
> 
> thanks,
> julia

Re: lustre: question about lov_request.c

[Julia Lawall]

On Sun, 7 Jun 2015, Drokin, Oleg wrote:

> Hello!
> 
>   You are right, set_pga seems to be a dead member. It was alive a once, but somehow not fully removed now,
>   so it's safe to drop the whole if and also the struct member itself.
>   set_oabufs could be dropped as well.

Thanks. I will do that.

julia

> 
>   Thanks.
> 
> Bye,
>     Oleg
> On Jun 7, 2015, at 4:11 PM, Julia Lawall wrote:
> 
> > Hello,
> > 
> > The function lov_finish_set in 
> > drivers/staging/lustre/lustre/lov/lov_request.c contains the code:
> > 
> >       if (set->set_pga) {
> >                int len = set->set_oabufs * sizeof(*set->set_pga);
> >                OBD_FREE_LARGE(set->set_pga, len);
> >        }
> > 
> > If I change the call to OBD_FREE_LARGE to kvfree, then len is not useful 
> > any more.  But actually, at least with grep, I can't find anywhere that 
> > either the set_pga field or the set_oabufs field is set.  Am I missing 
> > something, or can the whole if be removed?  Can these two fields go too?
> > 
> > thanks,
> > julia
> 
> 

Re: lustre: question about lov_request.c

[Julia Lawall]

>   You are right, set_pga seems to be a dead member. It was alive a once, but somehow not fully removed now,
>   so it's safe to drop the whole if and also the struct member itself.
>   set_oabufs could be dropped as well.

Looking further, in the same function I also don't see any other uses of
the tested field in:

                if (req->rq_oi.oi_md)
                        OBD_FREE_LARGE(req->rq_oi.oi_md, req->rq_buflen);

        if (set->set_lockh)
		lov_llh_put(set->set_lockh);

Can these be dropped as well?

thanks,
julia

Re: lustre: question about lov_request.c

[Drokin, Oleg]

On Jun 8, 2015, at 3:24 AM, Julia Lawall wrote:

>>  You are right, set_pga seems to be a dead member. It was alive a once, but somehow not fully removed now,
>>  so it's safe to drop the whole if and also the struct member itself.
>>  set_oabufs could be dropped as well.
> 
> Looking further, in the same function I also don't see any other uses of
> the tested field in:
> 
>                if (req->rq_oi.oi_md)
>                        OBD_FREE_LARGE(req->rq_oi.oi_md, req->rq_buflen);
> 
>        if (set->set_lockh)
> 		lov_llh_put(set->set_lockh);
> 
> Can these be dropped as well?

Yes, these two seems to be on their way out too, so please feel free to remove them.

Thanks.

Bye,
    Oleg

lustre: LIBCFS_ALLOC

[Julia Lawall]

It seems that libcfs_kvzalloc doesn't use any particular threshold or 
switchingbetween kzalloc and vmalloc, so can be replaced by ths function 
too?

thanks,
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: lustre: LIBCFS_ALLOC

[Drokin, Oleg]

On Jun 23, 2015, at 2:23 AM, Julia Lawall wrote:

> It seems that libcfs_kvzalloc doesn't use any particular threshold or 
> switchingbetween kzalloc and vmalloc, so can be replaced by ths function 
> too?

If you mean to replace all instances of LIBCFS_ALLOC with libcfs_kvzalloc (and all frees with kvfree) then yes, that could be done I think.

Bye,
    Oleg