Hi,
On Wed, 2010-07-28 at 08:39 -0700, Linus Torvalds wrote:
> On Wed, Jul 28, 2010 at 4:15 AM, Steven Whitehouse <swhit...@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.
