linux-kernel - Re: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc and kfree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 1 May 2015 20:12:44 +0000
From:	"Drokin, Oleg" <oleg.drokin@...el.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
CC:	"Simmons, James A." <simmonsja@...l.gov>,
	Julia Lawall <Julia.Lawall@...6.fr>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"HPDD-discuss@...ts.01.org" <HPDD-discuss@...1.01.org>
Subject: Re: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc
 and kfree


On May 1, 2015, at 4:02 PM, Dan Carpenter wrote:

> We are hopefully going to get rid of OBD_ALLOC_LARGE() as well, though.
> 
> It's simple enough to write a function:
> 
> void *obd_zalloc(size_t size)
> {
> 	if (size > 4 * PAGE_CACHE_SIZE)
> 		return vzalloc(size);
> 	else
> 		return kmalloc(size, GFP_NOFS);

kzalloc here too. Except e also want to have locality of allocations.

> }
> 
> Except, huh?  Shouldn't we be using GFP_NOFS for the vzalloc() side?
> There was some discussion of that GFP_NOFS was a bit buggy back in 2010
> (http://marc.info/?l=linux-mm&m=128942194520631&w=4) but the current
> lustre code doesn't try to pass GFP_NOFS.

The patch I submitted was rejected, or so I think to remember, because we use __vmalloc_node
or something and it's not an exported symbol.
http://www.spinics.net/lists/linux-mm/msg83997.html

> Then it's simple enough to change OBD_FREE_LARGE() to kvfree().
> 
> Also it's weird that only the lustre people have thought of this trick
> to allocate big chunks of RAM and no one else has.  What would happen if
> we just change vmalloc() so it worked this way for everyone?

We are certainly not alone.
I saw this in a few other pieces of code.

void *ext4_kvmalloc(size_t size, gfp_t flags)
{
        void *ret;

        ret = kmalloc(size, flags | __GFP_NOWARN);
        if (!ret)
                ret = __vmalloc(size, flags, PAGE_KERNEL);
        return ret;
}

or kmem_zalloc_large in xfs.

The difference at hand is that we pessimistically assume anything over certain threshold would
fail in kmalloc anyway and others actually do try kmalloc and only switch to vmalloc if kmaloc failed.
Considerign how expensive (and unsafe) vmalloc is, there might be some benefit to converting to their
way of doing things too.

Bye,
    Oleg--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/