Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 13 Sep 2019 10:00:13 +0100 | Subject | Re: [RFC] Improve memset |
| |
On Fri, Sep 13, 2019 at 8:22 AM Borislav Petkov <bp@alien8.de> wrote: > > since the merge window is closing in and y'all are on a conference, I > thought I should take another stab at it. It being something which Ingo, > Linus and Peter have suggested in the past at least once. > > Instead of calling memset: > > ffffffff8100cd8d: e8 0e 15 7a 00 callq ffffffff817ae2a0 <__memset> > > and having a JMP inside it depending on the feature supported, let's simply > have the REP; STOSB directly in the code:
That's probably fine for when the memset *is* a call, but:
> The result is this: > > static __always_inline void *memset(void *dest, int c, size_t n) > { > void *ret, *dummy; > > asm volatile(ALTERNATIVE_2_REVERSE("rep; stosb",
Forcing this code means that if you do
struct { long hi, low; } a; memset(&a, 0, sizeof(a));
you force that "rep stosb". Which is HORRID.
The compiler should turn it into just one single 8-byte store. But because you took over all of memset(), now that doesn't happen.
In fact, the compiler should be able to keep a structure like that in registers if the use of it is fairly simple. Which again wouldn't happen due to forcing that inline asm.
And "rep movsb" is ok for variable-sized memsets (well, honestly, generally only when size is sufficient, but it's been getting progressively better). But "rep movsb" is absolutely disastrous for small constant-sized memset() calls. It serializes the pipeline, it takes tens of cycles etc - for something that can take one single cycle and be easily hidden in the instruction stream among other changes.
And we do have a number of small structs etc in the kernel.
So we do need to have gcc do the __builtin_memset() for the simple cases..
Linus
| |