> -----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? When _set_range() calls with NULL, it is called from mark_written_sectors(). For each sector, we must first mark it init in bl_mark_sectors_init() before marking it written. And memory for the entry is allocated in bl_mark_sectors_init(). So if we don't find the entry somehow in mark_written_sectors(), there must be some bug out there. That is the BUG_ON added for. Thanks, Tao > > > 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. > > 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
