[Cluster-devel] GFS2: Use kmalloc when possible for ->readdir()

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Wed, 2010-07-28 at 08:39 -0700, Linus Torvalds wrote:
> On Wed, Jul 28, 2010 at 4:15 AM, Steven Whitehouse <swhiteho@redhat.com> wrote:
> >
> > We may be able to eliminate vmalloc entirely at some stage,
> > but this is easy to do right away.
> 
> Quite frankly, I'd much rather see this abstracted out a bit. Why not just do a
> 
>   void *memalloc(unsigned int size)
>   {
>       if (size < KMALLOC_MAX_SIZE) {
>          void *ptr = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
>          if (ptr)
>             return ptr;
>       }
>       return vmalloc(size);
>    }
> 
>    void memfree(void *ptr)
>    {
>       unsigned long addr = (unsigned long) ptr;
> 
>       if (is_vmalloc_addr(addr)) {
>          vfree(ptr);
>          return;
>       }
> 
>       kfree(ptr);
>    }
> 
> wouldn't that be much nicer? No need for that explicit flag, and you
> don't mess up an already way-too-ugly function even more.
> 
> Also, I do notice that you used GFP_NOFS, but you didn't use that for
> the vmalloc() thing. If there really are lock reentrancy reasons, you
> _could_ use __vmalloc(size, GFP_NOFS, PAGE_KERNEL).  But since you've
> been using vmalloc() for a long time, I suspect GFP_KERNEL works fine.
> Yes/no?
> 
>                       Linus

Well it had been working ok, but I don't really trust it since we are
holding a glock on the directory in shared mode at this point. That
means that if we (as a result of dropping dcache) need to unlink an
inode, we might land up taking glocks in the wrong order.

We use GFP_NOFS everywhere else under glocks for that reason, so I think
its safer to use GFP_NOFS here.

An updated patch based on your comments follows in the next email,

Steve.