linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Matthew Wilcox <willy@infradead.org>, linux-mm@kvack.org
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>,
	Matthew Wilcox <mawilcox@microsoft.com>,
	linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [PATCH 3/4] mm: Add free()
Date: Fri, 23 Mar 2018 09:04:10 +0100	[thread overview]
Message-ID: <1e95ce64-828b-1214-a930-1ffaedfa00b8@rasmusvillemoes.dk> (raw)
In-Reply-To: <20180322195819.24271-4-willy@infradead.org>

On 2018-03-22 20:58, Matthew Wilcox wrote:
> From: Matthew Wilcox <mawilcox@microsoft.com>
> 
> free() can free many different kinds of memory.

I'd be a bit worried about using that name. gcc very much knows about
the C standard's definition of that function, as can be seen on
godbolt.org by compiling

void free(const void *);
void f(void)
{
    free((void*)0);
}

with -O2 -Wall -Wextra -c. Anything from 4.6 onwards simply compiles this to

f:
 repz retq

And sure, your free() implementation obviously also has that property,
but I'm worried that they might one day decide to warn about the
prototype mismatch (actually, I'm surprised it doesn't warn now, given
that it obviously pretends to know what free() function I'm calling...),
or make some crazy optimization that will break stuff in very subtle ways.

Also, we probably don't want people starting to use free() (or whatever
name is chosen) if they do know the kind of memory they're freeing?
Maybe it should not be advertised that widely (i.e., in kernel.h).

Rasmus

  reply	other threads:[~2018-03-23  8:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-22 19:58 [PATCH 0/4] Add free() function Matthew Wilcox
2018-03-22 19:58 ` [PATCH 1/4] decompression: Rename malloc and free Matthew Wilcox
2018-03-22 19:58 ` [PATCH 2/4] Rename 'free' functions Matthew Wilcox
2018-03-22 19:58 ` [PATCH 3/4] mm: Add free() Matthew Wilcox
2018-03-23  8:04   ` Rasmus Villemoes [this message]
2018-03-23 14:34     ` Matthew Wilcox
2018-04-03  8:50       ` Pavel Machek
2018-04-03 11:41         ` Matthew Wilcox
2018-03-23 13:33   ` Kirill Tkhai
2018-03-23 15:14     ` Matthew Wilcox
2018-03-23 15:49       ` Kirill Tkhai
2018-03-23 16:15       ` Matthew Wilcox
2018-03-25 23:56       ` Matthew Wilcox
2018-03-24  7:38   ` kbuild test robot
2018-03-22 19:58 ` [PATCH 4/4] rcu: Switch to using free() instead of kfree() Matthew Wilcox
2018-03-24  7:07   ` kbuild test robot
2018-03-24  8:20   ` kbuild test robot

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=1e95ce64-828b-1214-a930-1ffaedfa00b8@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mawilcox@microsoft.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=willy@infradead.org \
    /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).