linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Julia Lawall <Julia.Lawall@lip6.fr>
Cc: Oleg Drokin <oleg.drokin@intel.com>,
	devel@driverdev.osuosl.org,
	Andreas Dilger <andreas.dilger@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	HPDD-discuss@ml01.01.org
Subject: Re: [PATCH] staging: lustre: llite: Use kzalloc and rewrite null tests
Date: Fri, 19 Sep 2014 02:43:09 +0300	[thread overview]
Message-ID: <20140918234309.GP17875@mwanda> (raw)
In-Reply-To: <1411071842-24714-2-git-send-email-Julia.Lawall@lip6.fr>

On Thu, Sep 18, 2014 at 10:24:02PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> This patch removes some kzalloc-related macros and rewrites the
> associated null tests to use !x rather than x == NULL.
> 

This is sort of exactly what Oleg asked us not to do in his previous
email.  ;P

I think there might be ways to get good enough tracing using standard
kernel features but it's legitimately tricky to update everything to
using mempools or whatever.  Maybe we should give Oleg some breathing
room to do this.

I hate looking at the OBD_ALLOC* macros, but really it's not as if we
don't have allocation helper functions anywhere in the rest of the
kernel.  It's just that the style of the lustre helpers is so very very
ugly.  It took me a while to spot that OBD_ALLOC() zeroes memory, for
example.

It should be relatively easy to re-write the macros so we can change
formats like this:

old:	OBD_ALLOC(ptr, size);
new:	ptr = obd_zalloc(size, GFP_NOFS);

old:	OBD_ALLOC_WAIT(ptr, size);
new:	ptr = obd_zalloc(size, GFP_KERNEL);

old:	OBD_ALLOC_PTR(ptr);
new:	ptr = obd_zalloc(sizeof(*ptr), GFP_NOFS);

etc...

Writing it this way means that we can't put the name of the pointer
we're allocating in the debug output but we could use the file and line
number instead or something.  Oleg, what do you think?

If we decide to mass convert to standard functions later then it's dead
simple to do that with sed.

The __OBD_MALLOC_VERBOSE() is hard to read.  It has side effect bugs if
you try to call OBD_ALLOC(ptr++, size);  The kernel already has a way to
inject kmalloc() failures for a specific module so that bit can be
removed.  Read Documentation/fault-injection/fault-injection.txt

regards,
dan carpenter

  reply	other threads:[~2014-09-18 23:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 20:24 [PATCH] Use kzalloc and rewrite null tests Julia Lawall
2014-09-18 20:24 ` [PATCH] staging: lustre: llite: " Julia Lawall
2014-09-18 23:43   ` Dan Carpenter [this message]
2014-09-19  2:57     ` Drokin, Oleg
2014-09-19  3:04       ` [HPDD-discuss] " Drokin, Oleg
2014-09-19  4:45         ` Julia Lawall
2014-09-19 13:36           ` Drokin, Oleg
2014-09-19 13:50             ` Julia Lawall
2014-09-19  9:11       ` Dan Carpenter
2014-09-19 13:27         ` Drokin, Oleg

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=20140918234309.GP17875@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=HPDD-discuss@ml01.01.org \
    --cc=Julia.Lawall@lip6.fr \
    --cc=andreas.dilger@intel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).