RE: [PATCH 6/8] pnfsblock: clean up _add_entry

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Benny Halevy [mailto:bhalevy@xxxxxxxxxx]
> Sent: Thursday, November 10, 2011 4:44 PM
> To: Peng Tao
> Cc: linux-nfs@xxxxxxxxxxxxxxx; Trond.Myklebust@xxxxxxxxxx; Peng, Tao
> Subject: Re: [PATCH 6/8] pnfsblock: clean up _add_entry
> 
> On 2011-11-09 17:16, Peng Tao wrote:
> > It is wrong to kmalloc in _add_entry() as it is inside
> > spinlock. memory should be already allocated _add_entry() is called.
> 
> What about the call from _set_range, it is passing NULL?
> 
> > So BUG instead if we fail to find a match and no memory preallocated.
> >
> > Signed-off-by: Peng Tao <peng_tao@xxxxxxx>
> > ---
> >  fs/nfs/blocklayout/extents.c |    9 ++-------
> >  1 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/nfs/blocklayout/extents.c b/fs/nfs/blocklayout/extents.c
> > index f383524..952ea8a 100644
> > --- a/fs/nfs/blocklayout/extents.c
> > +++ b/fs/nfs/blocklayout/extents.c
> > @@ -110,13 +110,8 @@ static int _add_entry(struct my_tree *tree, u64 s, int32_t
> tag,
> >  		return 0;
> >  	} else {
> >  		struct pnfs_inval_tracking *new;
> > -		if (storage)
> > -			new = storage;
> > -		else {
> > -			new = kmalloc(sizeof(*new), GFP_NOFS);
> > -			if (!new)
> > -				return -ENOMEM;
> > -		}
> > +		BUG_ON(!storage);
> 
> The BUG isn't needed as you dereference the pointer right away.
Yes. Will remove it.

Thanks a lot for reviewing!
Tao

> 
> Benny
> 
> > +		new = storage;
> >  		new->it_sector = s;
> >  		new->it_tags = (1 << tag);
> >  		list_add(&new->it_link, &pos->it_link);

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux