All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Drokin, Oleg" <oleg.drokin@intel.com>
Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	"HPDD-discuss@lists.01.org" <HPDD-discuss@ml01.01.org>,
	"Simmons, James A." <simmonsja@ornl.gov>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc and kfree
Date: Fri, 01 May 2015 20:58:35 +0000	[thread overview]
Message-ID: <20150501205835.GA2692@kroah.com> (raw)
In-Reply-To: <CA8D4D3B-39CB-4DE4-835D-047692A7E5DE@intel.com>

On Fri, May 01, 2015 at 08:52:37PM +0000, Drokin, Oleg wrote:
> 
> On May 1, 2015, at 4:49 PM, Greg Kroah-Hartman wrote:
> 
> > On Fri, May 01, 2015 at 08:36:05PM +0000, Simmons, James A. 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);
> >>> }
> >>> 
> >>> 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\x128942194520631&w=4) but the current
> >>> lustre code doesn't try to pass GFP_NOFS.
> >> 
> >> The version in the upstream client is out of date. The current macro in the Intel master
> >> Branch is:
> > 
> > That's not helpful at all, why do we even have an in-kernel version of
> > this code if you don't do your development in the kernel?
> > 
> > Please sync with the kernel tree very soon, or I'm just going to delete
> > this whole thing.  This is getting _really_ frustrating.
> 
> The patch was submitted.
> But it depends on a symbol that's not exported.
> I was not able to change that.
> http://www.spinics.net/lists/linux-mm/msg83997.html

But you were given a hint on how to change that :)

Anyway, I'd recommend switching to what ext4 and xfs does, as you point
out in another email in this thread, it looks a lot better overall.

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Drokin, Oleg" <oleg.drokin@intel.com>
Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
	"kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	"HPDD-discuss@lists.01.org" <HPDD-discuss@ml01.01.org>,
	"Simmons, James A." <simmonsja@ornl.gov>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [HPDD-discuss] [PATCH 2/11] Staging: lustre: fld: Use kzalloc and kfree
Date: Fri, 1 May 2015 22:58:35 +0200	[thread overview]
Message-ID: <20150501205835.GA2692@kroah.com> (raw)
In-Reply-To: <CA8D4D3B-39CB-4DE4-835D-047692A7E5DE@intel.com>

On Fri, May 01, 2015 at 08:52:37PM +0000, Drokin, Oleg wrote:
> 
> On May 1, 2015, at 4:49 PM, Greg Kroah-Hartman wrote:
> 
> > On Fri, May 01, 2015 at 08:36:05PM +0000, Simmons, James A. 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);
> >>> }
> >>> 
> >>> 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 version in the upstream client is out of date. The current macro in the Intel master
> >> Branch is:
> > 
> > That's not helpful at all, why do we even have an in-kernel version of
> > this code if you don't do your development in the kernel?
> > 
> > Please sync with the kernel tree very soon, or I'm just going to delete
> > this whole thing.  This is getting _really_ frustrating.
> 
> The patch was submitted.
> But it depends on a symbol that's not exported.
> I was not able to change that.
> http://www.spinics.net/lists/linux-mm/msg83997.html

But you were given a hint on how to change that :)

Anyway, I'd recommend switching to what ext4 and xfs does, as you point
out in another email in this thread, it looks a lot better overall.

thanks,

greg k-h

  reply	other threads:[~2015-05-01 20:58 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-01 15:51 [PATCH 0/11] Use kzalloc and kfree Julia Lawall
2015-05-01 15:51 ` Julia Lawall
2015-05-01 15:51 ` [PATCH 11/11] staging: lustre: ptlrpc: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 10/11] staging: lustre: osc: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 9/11] staging: lustre: obdecho: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 8/11] staging: lustre: obdclass: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 18:30   ` walter harms
2015-05-01 18:30     ` walter harms
2015-05-01 18:42     ` Julia Lawall
2015-05-01 18:42       ` Julia Lawall
2015-05-01 15:51 ` [PATCH 7/11] staging: lustre: mgc: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 6/11] staging: lustre: mdc: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 5/11] staging: lustre: lmv: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 4/11] staging: lustre: ldlm: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 3/11] staging: lustre: lclient: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 15:51 ` [PATCH 2/11] Staging: lustre: fld: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-01 17:38   ` [HPDD-discuss] " Simmons, James A.
2015-05-01 17:38     ` Simmons, James A.
2015-05-01 17:48     ` Julia Lawall
2015-05-01 17:48       ` Julia Lawall
2015-05-01 18:49       ` Drokin, Oleg
2015-05-01 18:49         ` Drokin, Oleg
2015-05-01 20:18       ` Simmons, James A.
2015-05-01 20:18         ` Simmons, James A.
2015-05-01 20:47         ` Greg Kroah-Hartman
2015-05-01 20:47           ` Greg Kroah-Hartman
2015-05-01 22:57           ` Simmons, James A.
2015-05-01 22:57             ` Simmons, James A.
2015-05-01 20:49         ` Drokin, Oleg
2015-05-01 20:49           ` Drokin, Oleg
2015-05-01 22:59           ` Simmons, James A.
2015-05-01 22:59             ` Simmons, James A.
2015-05-01 18:05     ` Greg Kroah-Hartman
2015-05-01 18:05       ` Greg Kroah-Hartman
2015-05-01 20:02     ` Dan Carpenter
2015-05-01 20:02       ` Dan Carpenter
2015-05-01 20:12       ` Drokin, Oleg
2015-05-01 20:12         ` Drokin, Oleg
2015-05-01 20:36       ` Simmons, James A.
2015-05-01 20:36         ` Simmons, James A.
2015-05-01 20:49         ` Greg Kroah-Hartman
2015-05-01 20:49           ` Greg Kroah-Hartman
2015-05-01 20:52           ` Drokin, Oleg
2015-05-01 20:52             ` Drokin, Oleg
2015-05-01 20:58             ` Greg Kroah-Hartman [this message]
2015-05-01 20:58               ` Greg Kroah-Hartman
2015-05-01 21:13               ` Drokin, Oleg
2015-05-01 21:13                 ` Drokin, Oleg
2015-05-02  6:02                 ` Julia Lawall
2015-05-02  6:02                   ` Julia Lawall
2015-05-02  8:14                 ` Dan Carpenter
2015-05-02  8:14                   ` Dan Carpenter
2015-05-02  9:05                   ` Julia Lawall
2015-05-02  9:05                     ` Julia Lawall
2015-05-03  3:12                   ` Drokin, Oleg
2015-05-03  3:12                     ` Drokin, Oleg
2015-05-02  1:18     ` Simmons, James A.
2015-05-02  8:26     ` 'Greg Kroah-Hartman'
2015-05-02 17:17     ` Drokin, Oleg
2015-05-02 17:33     ` Greg Kroah-Hartman
2015-05-04 14:07     ` Simmons, James A.
2015-05-04 15:01     ` Dan Carpenter
2015-05-04 21:26     ` 'Greg Kroah-Hartman'
2015-05-01 15:51 ` [PATCH 1/11] staging: lustre: fid: " Julia Lawall
2015-05-01 15:51   ` Julia Lawall
2015-05-03 18:17 ` [PATCH 0/11] " Greg Kroah-Hartman
2015-05-03 18:17   ` Greg Kroah-Hartman
2015-05-03 18:39   ` Julia Lawall
2015-05-03 18:39     ` 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=20150501205835.GA2692@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=HPDD-discuss@ml01.01.org \
    --cc=Julia.Lawall@lip6.fr \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg.drokin@intel.com \
    --cc=simmonsja@ornl.gov \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.