memset as memzero

memset as memzero

[Cyrill Gorcunov]

Hi list,

could anyone tell me why there is no official memzero function (or macros) in
the kernel. As I see a lot of  kernel parts calls for it (defying own macros
as alias to memset). Maybe there is a special reason not to do so? Actually
my suggestion is to define _one_ general macros for this.

		Cyrill

Re: memset as memzero

[Robert P. J. Day]

On Sat, 22 Sep 2007, Cyrill Gorcunov wrote:

> Hi list,
>
> could anyone tell me why there is no official memzero function (or
> macros) in the kernel. As I see a lot of kernel parts calls for it
> (defying own macros as alias to memset). Maybe there is a special
> reason not to do so? Actually my suggestion is to define _one_
> general macros for this.

i brought up this issue on the KJ list once upon a time:

https://lists.linux-foundation.org/pipermail/kernel-janitors/2007-February/017847.html

and there didn't seem to be much enthusiasm for it.

  however, i am still curious why there isn't more use of the
already-defined "clear_page" macro.  most architectures appear to
define it:

$ grep -r "define.*clear_page" include

but there are still numerous explicit calls to memset() to zero a
chunk of memory that is exactly PAGE_SIZE in size.  just an
observation.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

Re: memset as memzero

[Cyrill Gorcunov]

[Robert P. J. Day - Sat, Sep 22, 2007 at 04:48:28AM -0400]
| On Sat, 22 Sep 2007, Cyrill Gorcunov wrote:
| 
| > Hi list,
| >
| > could anyone tell me why there is no official memzero function (or
| > macros) in the kernel. As I see a lot of kernel parts calls for it
| > (defying own macros as alias to memset). Maybe there is a special
| > reason not to do so? Actually my suggestion is to define _one_
| > general macros for this.
| 
| i brought up this issue on the KJ list once upon a time:
| 
| https://lists.linux-foundation.org/pipermail/kernel-janitors/2007-February/017847.html
| 
| and there didn't seem to be much enthusiasm for it.
| 
|   however, i am still curious why there isn't more use of the
| already-defined "clear_page" macro.  most architectures appear to
| define it:
| 
| $ grep -r "define.*clear_page" include
| 
| but there are still numerous explicit calls to memset() to zero a
| chunk of memory that is exactly PAGE_SIZE in size.  just an
| observation.
| 
| rday
| -- 
| ========================================================================
| Robert P. J. Day
| Linux Consulting, Training and Annoying Kernel Pedantry
| Waterloo, Ontario, CANADA
| 
| http://crashcourse.ca
| ========================================================================
| 

Thanks Robert for the answer, I'll mark this (clear_page) in my "must to
take a look" list ;)
Well if there is no strong reason of keeping this separate '#define memzero'
I think it's a good case to merge them in some _single_ #define ;)

Waiting for other comments...

P.S.
In a mail you pointed to said that memset(...,0,...) is quite clear -
yes it's quite clear indeed but we already _have_ a lot of '#define memzero'
and who knows or may give the guarantee that new '#define memzero '_will not_'
appear in the kernel.

		Cyrill

Re: memset as memzero

[Robert P. J. Day]

On Sat, 22 Sep 2007, Cyrill Gorcunov wrote:

> Thanks Robert for the answer, I'll mark this (clear_page) in my
> "must to take a look" list ;)

there's already been a discussion about clear_page() as well:

  http://lists.openwall.net/linux-kernel/2006/12/29/39

you might want to start there to get up to speed if you intend to
invest any time in this.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

Re: memset as memzero

[Cyrill Gorcunov]

[Robert P. J. Day - Sat, Sep 22, 2007 at 05:55:04AM -0400]
| On Sat, 22 Sep 2007, Cyrill Gorcunov wrote:
| 
| > Thanks Robert for the answer, I'll mark this (clear_page) in my
| > "must to take a look" list ;)
| 
| there's already been a discussion about clear_page() as well:
| 
|   http://lists.openwall.net/linux-kernel/2006/12/29/39
| 
| you might want to start there to get up to speed if you intend to
| invest any time in this.
| 
| rday
| -- 
| ========================================================================
| Robert P. J. Day
| Linux Consulting, Training and Annoying Kernel Pedantry
| Waterloo, Ontario, CANADA
| 
| http://crashcourse.ca
| ========================================================================
| 

thanks

		Cyrill

Re: memset as memzero

[Arjan van de Ven]

On Sat, 22 Sep 2007 12:33:55 +0400
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> Hi list,
> 
> could anyone tell me why there is no official memzero function (or
> macros) in the kernel.

it doesn't add value.... memset with a constant 0 is just as fast
(since the compiler knows it's 0) than any wrapper around it, and the
syntax around it is otherwise the same.


> As I see a lot of  kernel parts calls for it
> (defying own macros as alias to memset). Maybe there is a special
> reason not to do so? Actually my suggestion is to define _one_
> general macros for this.

my suggestion is to nuke all the macros and just use memset().

Re: memset as memzero

[Linus Torvalds]

On Sat, 22 Sep 2007, Arjan van de Ven wrote:
> 
> it doesn't add value.... memset with a constant 0 is just as fast
> (since the compiler knows it's 0) than any wrapper around it, and the
> syntax around it is otherwise the same.

Indeed.

The reason we have "clear_page()" is not because the value we're writing 
is constant - that doesn't really help/change anything at all. We could 
have had a "fill_page()" that sets the value to any random byte, it's just 
that zero is the only value that we really care about.

So the reason we have "clear_page()" is because the *size* and *alignment* 
is constant and known at compile time, and unlike the value you write, 
that actually matters.

So "memzero()" would never really make sense as anything but a syntactic 
wrapper around "memset(x,0,size)". 

			Linus

Re: memset as memzero

[Dave Jones]

On Sat, Sep 22, 2007 at 11:53:53AM -0700, Linus Torvalds wrote:
 > 
 > 
 > On Sat, 22 Sep 2007, Arjan van de Ven wrote:
 > > 
 > > it doesn't add value.... memset with a constant 0 is just as fast
 > > (since the compiler knows it's 0) than any wrapper around it, and the
 > > syntax around it is otherwise the same.
 > 
 > Indeed.
 > 
 > The reason we have "clear_page()" is not because the value we're writing 
 > is constant - that doesn't really help/change anything at all. We could 
 > have had a "fill_page()" that sets the value to any random byte, it's just 
 > that zero is the only value that we really care about.
 > 
 > So the reason we have "clear_page()" is because the *size* and *alignment* 
 > is constant and known at compile time, and unlike the value you write, 
 > that actually matters.
 > 
 > So "memzero()" would never really make sense as anything but a syntactic 
 > wrapper around "memset(x,0,size)". 

There is one useful argument for memzero (or bzero to give it its proper
name), and that's that it's impossible to screw up.
I'm still amazed at how many times I see

	memset (x,size,0);

in various code. So much so, that my editor highlights it now to spot
it during code review.  As does my mail client.  To be on the safe
side, I also have a cron job grepping for it in my ~/Mail/commits
for all the projects I'm interested in.

It's tragic really just how easy it is to screw it up.

	Dave

-- 
http://www.codemonkey.org.uk

Re: memset as memzero

[Arnaldo Carvalho de Melo]

Em Sun, Sep 23, 2007 at 11:32:43AM -0400, Dave Jones escreveu:
> On Sat, Sep 22, 2007 at 11:53:53AM -0700, Linus Torvalds wrote:
>  > 
>  > 
>  > On Sat, 22 Sep 2007, Arjan van de Ven wrote:
>  > > 
>  > > it doesn't add value.... memset with a constant 0 is just as fast
>  > > (since the compiler knows it's 0) than any wrapper around it, and the
>  > > syntax around it is otherwise the same.
>  > 
>  > Indeed.
>  > 
>  > The reason we have "clear_page()" is not because the value we're writing 
>  > is constant - that doesn't really help/change anything at all. We could 
>  > have had a "fill_page()" that sets the value to any random byte, it's just 
>  > that zero is the only value that we really care about.
>  > 
>  > So the reason we have "clear_page()" is because the *size* and *alignment* 
>  > is constant and known at compile time, and unlike the value you write, 
>  > that actually matters.
>  > 
>  > So "memzero()" would never really make sense as anything but a syntactic 
>  > wrapper around "memset(x,0,size)". 
> 
> There is one useful argument for memzero (or bzero to give it its proper
> name), and that's that it's impossible to screw up.
> I'm still amazed at how many times I see
> 
> 	memset (x,size,0);
> 
> in various code. So much so, that my editor highlights it now to spot
> it during code review.  As does my mail client.  To be on the safe
> side, I also have a cron job grepping for it in my ~/Mail/commits
> for all the projects I'm interested in.
> 
> It's tragic really just how easy it is to screw it up.

bzero! That is it, its nothing new, just a sane name to something that
is useful to humans, even being of sheer arrogant disdain for machines
as a useless stuff only humans couldn't get right. Yeah, us screw up
pretty much more than them.

- Arnaldo

Re: memset as memzero

[Linus Torvalds]

On Sun, 23 Sep 2007, Arnaldo Carvalho de Melo wrote:
> 
> bzero! That is it, its nothing new, just a sane name to something [..]

No, please no!

The BSD memory functions are nasty. If you do bzero, you logically should 
do the others too, and they are way inferior to the standard ones. Let's 
not go there.

Besides, if we want to avoid mistakes, I would suggest going to a much 
higher level. Ie more along the lines of also fixing the size and 
alignment, and using something like

	#define memclear(p) memset(p, 0, sizeof(*(p)))

because if you actually do something like

	git grep 'memset.*,[ 	]*0[ 	]*,'

(those [..] things contatain a space and a tab), you'll see that a *lot* 
of them share that pattern. 

Not that I think it's really worth it.

		Linus

Re: memset as memzero

[Willy Tarreau]

On Sun, Sep 23, 2007 at 10:05:05AM -0700, Linus Torvalds wrote:
> 
> 
> On Sun, 23 Sep 2007, Arnaldo Carvalho de Melo wrote:
> > 
> > bzero! That is it, its nothing new, just a sane name to something [..]
> 
> No, please no!
> 
> The BSD memory functions are nasty. If you do bzero, you logically should 
> do the others too, and they are way inferior to the standard ones. Let's 
> not go there.
> 
> Besides, if we want to avoid mistakes, I would suggest going to a much 
> higher level. Ie more along the lines of also fixing the size and 
> alignment, and using something like
> 
> 	#define memclear(p) memset(p, 0, sizeof(*(p)))

I don't like it when macros magically do sizeof(*p), because people often
think that the macro is smarter than it really is, and you commonly end
up with code looking like this :

     char *p;
     ...
     p = kmalloc(n);
     ...
     memclear(p);

This can happen for instance when replacing a stack-allocated buffer
with a malloc because it became too big for the stack. Such a mistake
is *very hard* to detect by human eye, while having "sizeof(*p)" in
the same function as "char *p" will trigger some automatisms in most
readers' brains.

> because if you actually do something like
> 
> 	git grep 'memset.*,[ 	]*0[ 	]*,'
> 
> (those [..] things contatain a space and a tab), you'll see that a *lot* 
> of them share that pattern. 

At least current code is still greppable for such usages. Doing too
much magics with macros often harms debugging. I could agree with
having a macro to force the pattern to '0', but not to force the size.

> Not that I think it's really worth it.

I don't think either.

Willy

Re: memset as memzero

[Robert P. J. Day]

On Sun, 23 Sep 2007, Dave Jones wrote:

> There is one useful argument for memzero (or bzero to give it its
> proper name), and that's that it's impossible to screw up. I'm still
> amazed at how many times I see
>
> 	memset (x,size,0);
>
> in various code. So much so, that my editor highlights it now to
> spot it during code review.  As does my mail client.  To be on the
> safe side, I also have a cron job grepping for it in my
> ~/Mail/commits for all the projects I'm interested in.

  taking a step back, regardless of what constitutes a sane versus
not-sane definition of a useful macro, i think a lot of the content of
kernel.h could be moved out of there and put in a more appropriate
header file called, say, macros.h.

  the first comment in kernel.h claims that

/*
 * 'kernel.h' contains some often-used function prototypes etc
 */

but there's buckets more stuff in there than just some function
prototypes.  macros for type limits, alignment, array sizes, rounding,
and on and on.  and as for those prototypes, is there any reason that
kernel.h includes them explicitly for the contents of lib/vsprintf.c
rather than just including, say, a hypothetical vsprintf.h?  just
curious.

in any case, it would seem that kernel.h could stand a good cleaning.
it give the impression of just being an arbitrary dumping ground when
folks can't figure out where to put something.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

Re: memset as memzero

[Arnaldo Carvalho de Melo]

Em Sun, Sep 23, 2007 at 12:33:13PM -0400, Robert P. J. Day escreveu:
> On Sun, 23 Sep 2007, Dave Jones wrote:
> 
> > There is one useful argument for memzero (or bzero to give it its
> > proper name), and that's that it's impossible to screw up. I'm still
> > amazed at how many times I see
> >
> > 	memset (x,size,0);
> >
> > in various code. So much so, that my editor highlights it now to
> > spot it during code review.  As does my mail client.  To be on the
> > safe side, I also have a cron job grepping for it in my
> > ~/Mail/commits for all the projects I'm interested in.
> 
>   taking a step back, regardless of what constitutes a sane versus
> not-sane definition of a useful macro, i think a lot of the content of
> kernel.h could be moved out of there and put in a more appropriate
> header file called, say, macros.h.
> 
>   the first comment in kernel.h claims that
> 
> /*
>  * 'kernel.h' contains some often-used function prototypes etc
>  */
> 
> but there's buckets more stuff in there than just some function
> prototypes.  macros for type limits, alignment, array sizes, rounding,
> and on and on.  and as for those prototypes, is there any reason that
> kernel.h includes them explicitly for the contents of lib/vsprintf.c
> rather than just including, say, a hypothetical vsprintf.h?  just
> curious.
> 
> in any case, it would seem that kernel.h could stand a good cleaning.
> it give the impression of just being an arbitrary dumping ground when
> folks can't figure out where to put something.

In an ideal world kernel.h has no place, I guess. I guess too that
janitors could make the world ideal in that respect. Keep moving things
from there to the right place. Don't do it every other day tho. People
find it annoying.

- Arnaldo

Re: memset as memzero

[Bernd Eckenfels]

In article <20070922124659.741cca42@laptopd505.fenrus.org> you wrote:
> it doesn't add value.... memset with a constant 0 is just as fast
> (since the compiler knows it's 0) than any wrapper around it, and the
> syntax around it is otherwise the same.

it would however allow easier changing if you need to add a page cleaning
system (for example new architecture which has support for it). On the other
hand, is memset used for anything else than zero filling or redzone
"filling"?

Gruss
Bernd

Re: memset as memzero

[Oleg Verych (nntp)]

22-09-2007, Bernd Eckenfels:
> In article <20070922124659.741cca42@laptopd505.fenrus.org> you wrote:
>> it doesn't add value.... memset with a constant 0 is just as fast
>> (since the compiler knows it's 0) than any wrapper around it, and the
>> syntax around it is otherwise the same.

linux/arch/x86_64/lib/memset.S isn't file for compile, but for
assembler, isn't it? Removing argument processing will have size effect.

> it would however allow easier changing if you need to add a page cleaning
> system (for example new architecture which has support for it).
[]

> On the other hand, is memset used for anything else than zero filling
> or redzone "filling"?

There are non zero ones (not sure what redzone is, though).

> Gruss
> Bernd
(Here are reply-to-all people, BTW)
_____

Re: memset as memzero

[Cyrill Gorcunov]

[Arjan van de Ven - Sat, Sep 22, 2007 at 12:46:59PM -0700]
| On Sat, 22 Sep 2007 12:33:55 +0400
| Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| 
| > Hi list,
| > 
| > could anyone tell me why there is no official memzero function (or
| > macros) in the kernel.
| 
| it doesn't add value.... memset with a constant 0 is just as fast
| (since the compiler knows it's 0) than any wrapper around it, and the
| syntax around it is otherwise the same.
| 

It seems I expressed wrong. I'm worried about code duplication. Look
simple grep for memzero tells us that in particular:

...
-- arch/x86_64/boot/compressed/misc.c:110:#define memzero(s, n) memset ((s), 0, (n))
-- init/do_mounts_rd.c:279:#define memzero(s, n)     memset ((s), 0, (n))
-- init/initramfs.c:377:#define memzero(s, n)     memset ((s), 0, (n))
-- lib/inflate.c:331:  memzero(stk->c, sizeof(stk->c));
...

So instead of several 'define' that are the _same_ maybe better just use
_single common_ define? That's all I wanna ask. (Btw, it seems ARM has
a special case for memzero ;)

| 
| > As I see a lot of  kernel parts calls for it
| > (defying own macros as alias to memset). Maybe there is a special
| > reason not to do so? Actually my suggestion is to define _one_
| > general macros for this.
| 
| my suggestion is to nuke all the macros and just use memset().
| 

Quite clear, thanks. So if that is OK - I'm shutting up ;)

		Cyrill