public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Dilger, Andreas" <andreas.dilger@intel.com>
To: Julia Lawall <julia.lawall@lip6.fr>
Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Drokin, Oleg" <oleg.drokin@intel.com>,
	"lustre-devel@lists.lustre.org" <lustre-devel@lists.lustre.org>
Subject: Re: [lustre-devel] LIBCFS_ALLOC
Date: Tue, 30 Jun 2015 21:26:43 +0000	[thread overview]
Message-ID: <D1B863B8.FA7C0%andreas.dilger@intel.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1506280844460.2103@localhost6.localdomain6>

On 2015/06/28, 12:52 AM, "Julia Lawall" <julia.lawall@lip6.fr> wrote:

>It is not clear that all of the uses of LIBCFS_ALLOC really risk needing
>vmalloc.  For example:
>
>lnet/klnds/socklnd/socklnd.c, function ksocknal_accept:
>
>ksock_connreq_t *cr;
>...
>LIBCFS_ALLOC(cr, sizeof(*cr));
>
>The definition of ksock_connreq_t is:
>
>typedef struct ksock_connreq {
>        struct list_head ksncr_list;  /* stash on ksnd_connd_connreqs */
>        lnet_ni_t        *ksncr_ni;   /* chosen NI */
>        struct socket    *ksncr_sock; /* accepted socket */
>} ksock_connreq_t;
>
>This looks like a very small structure.
>
>LIBCFS_ALLOC relies on a test on the size, which should be able to be
>compiled away.  libcfs_kvzalloc on the other hand relies on the failure
>of 
>kmalloc and so the test for that won't be compiled away.

There are probably only a handful of places where trying vmalloc() even
makes sense.  In most cases, LIBCFS_ALLOC() can be replaced by a straight
call to kmalloc() because the allocation size is small enough, and only a
few need to use libcfs_kvzalloc().  Anything at PAGE_SIZE or less will
either work with kmalloc() or it won't work at all.

I do agree with James' comments that vmalloc() was needed for both very
large allocations that can't be satisfied by kmalloc() at all, as well as
smaller allocations (anything over a couple of pages) due to fragmentation
of free pages after running for a long time.  I think libcfs_kvzalloc() is
at least as good as the size-based threshold used previously, since using
kmalloc() is going to be faster than vmalloc() and would work better on
32-bit platforms with limited vmalloc() size.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division



  parent reply	other threads:[~2015-06-30 21:26 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-20 16:58 [PATCH 00/12] Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:58 ` [PATCH 01/12] staging: lustre: fid: " Julia Lawall
2015-06-23  8:25   ` Dilger, Andreas
2015-06-23  9:23     ` Dan Carpenter
2015-06-23  9:35       ` Julia Lawall
2015-06-23  9:57         ` Dan Carpenter
2015-06-23 10:51           ` Julia Lawall
2015-06-24 20:14             ` [lustre-devel] " Simmons, James A.
2015-06-23 22:03           ` Joe Perches
2015-06-23 22:11       ` Joe Perches
2015-06-28  6:52     ` LIBCFS_ALLOC Julia Lawall
2015-06-28 21:54       ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 14:56         ` LIBCFS_ALLOC Simmons, James A.
2015-06-30 15:01           ` LIBCFS_ALLOC Julia Lawall
2015-07-02 22:25             ` [lustre-devel] LIBCFS_ALLOC Simmons, James A.
2015-07-03 11:52               ` Dilger, Andreas
2015-06-30 17:38           ` LIBCFS_ALLOC Dan Carpenter
2015-06-30 21:26       ` Dilger, Andreas [this message]
2015-06-20 16:59 ` [PATCH 02/12] staging: lustre: fld: Use !x to check for kzalloc failure Julia Lawall
2015-06-20 16:59 ` [PATCH 03/12] staging: lustre: lclient: " Julia Lawall
2015-06-20 16:59 ` [PATCH 04/12] staging: lustre: ldlm: " Julia Lawall
2015-06-20 16:59 ` [PATCH 05/12] staging: lustre: lmv: " Julia Lawall
2015-06-20 16:59 ` [PATCH 06/12] staging: lustre: lov: " Julia Lawall
2015-06-20 16:59 ` [PATCH 07/12] staging: lustre: mdc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 08/12] staging: lustre: mgc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 09/12] staging: lustre: obdclass: " Julia Lawall
2015-06-21 10:02   ` walter harms
2015-06-21 10:29     ` Julia Lawall
2015-06-20 16:59 ` [PATCH 10/12] staging: lustre: obdecho: " Julia Lawall
2015-06-20 16:59 ` [PATCH 11/12] staging: lustre: osc: " Julia Lawall
2015-06-20 16:59 ` [PATCH 12/12] staging: lustre: ptlrpc: " Julia Lawall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D1B863B8.FA7C0%andreas.dilger@intel.com \
    --to=andreas.dilger@intel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=julia.lawall@lip6.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=oleg.drokin@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox