memset as memzero
Cyrill Gorcunov
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Robert P. J. Day
> 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
========================================================================
Cyrill Gorcunov
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
Robert P. J. Day
> 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
========================================================================
Cyrill Gorcunov
| 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
Arjan van de Ven
Cyrill Gorcunov <gorc...@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().
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
Bernd Eckenfels
> 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
Cyrill Gorcunov
| On Sat, 22 Sep 2007 12:33:55 +0400
| Cyrill Gorcunov <gorc...@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
Oleg Verych (nntp)
> In article <20070922124...@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)
_____
Dave Jones
>
>
> 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
Arnaldo Carvalho de Melo
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
Robert P. J. Day
> 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
========================================================================
Arnaldo Carvalho de Melo
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
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
Willy Tarreau
>
>
> 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