x86-64: memset()/memcpy() not fully standards compliant

x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

Forever these two functions have been limited to deal with at most 4G
at a time. While I cannot point out an in-tree user that would require
larger sizes, it is now the second time that within our Xen kernel we got
bitten by that limitation. Would you nevertheless accept a patch to
eliminate those shortcomings (iirc there may need to be workarounds
for CPU bugs when it comes to using string instructions on such large
blocks, albeit memmove() doesn't seem to care)?

Otherwise, is there any rationale for this sort of lurking bug?

Thanks, Jan

Re: x86-64: memset()/memcpy() not fully standards compliant

[hpanvin@gmail.com]

Yes, I think it is probably Just A Bug.  Unless it can be shown it makes it much slower, let's fix.

Jan Beulich <JBeulich@suse.com> wrote:

>Forever these two functions have been limited to deal with at most 4G
>at a time. While I cannot point out an in-tree user that would require
>larger sizes, it is now the second time that within our Xen kernel we
>got
>bitten by that limitation. Would you nevertheless accept a patch to
>eliminate those shortcomings (iirc there may need to be workarounds
>for CPU bugs when it comes to using string instructions on such large
>blocks, albeit memmove() doesn't seem to care)?
>
>Otherwise, is there any rationale for this sort of lurking bug?
>
>Thanks, Jan

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

On Thu, Jan 05, 2012 at 02:04:30PM +0000, Jan Beulich wrote:
> Forever these two functions have been limited to deal with at most 4G
> at a time. While I cannot point out an in-tree user that would require
> larger sizes, it is now the second time that within our Xen kernel we got
> bitten by that limitation. Would you nevertheless accept a patch to
> eliminate those shortcomings (iirc there may need to be workarounds
> for CPU bugs when it comes to using string instructions on such large
> blocks, albeit memmove() doesn't seem to care)?
> 
> Otherwise, is there any rationale for this sort of lurking bug?

Most (all?) of the CPUs I cared about when writing that code had 
bugs with string instructions and >4GB.

And I don't think anything in the kernel would ever process that big
areas in a single operation.

So it never seemed worth caring about.

You probably would need at least one more CMP & conditional jump
in the hot path, so everyone would pay for something that never happens.

-Andi

Re: x86-64: memset()/memcpy() not fully standards compliant

[H. Peter Anvin]

On 01/05/2012 10:28 AM, Andi Kleen wrote:
>>
>> Otherwise, is there any rationale for this sort of lurking bug?
> 
> Most (all?) of the CPUs I cared about when writing that code had 
> bugs with string instructions and >4GB.
> 

Is that still true, and do we even use string instructions still on
those old CPUs?  Jan's fixes don't introduce any additional delays in
the non-string-instruction paths.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

On Thu, Jan 05, 2012 at 03:22:52PM -0800, H. Peter Anvin wrote:
> On 01/05/2012 10:28 AM, Andi Kleen wrote:
> >>
> >> Otherwise, is there any rationale for this sort of lurking bug?
> > 
> > Most (all?) of the CPUs I cared about when writing that code had 
> > bugs with string instructions and >4GB.
> > 
> 
> Is that still true, and do we even use string instructions still on
> those old CPUs?  Jan's fixes don't introduce any additional delays in
> the non-string-instruction paths.

Yes various of the CPUs with bugs used string instructions.

Don't know the state on current CPUs.

Both string and non string instructions are used on modern CPUs,
so making any of that slower is not a good idea.

-Andi

Re: x86-64: memset()/memcpy() not fully standards compliant

[H. Peter Anvin]

On 01/05/2012 05:47 PM, Andi Kleen wrote:
>>
>> Is that still true, and do we even use string instructions still on
>> those old CPUs?  Jan's fixes don't introduce any additional delays in
>> the non-string-instruction paths.
> 
> Yes various of the CPUs with bugs used string instructions.
> 

Which CPUs are you talking about here?

> 
> Both string and non string instructions are used on modern CPUs,
> so making any of that slower is not a good idea.
> 

Obviously not, but I'm perfectly fine turning REP_GOOD off on old broken
CPUs.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

>>> On 06.01.12 at 03:03, "H. Peter Anvin" <hpa@zytor.com> wrote:
> On 01/05/2012 05:47 PM, Andi Kleen wrote:
>>>
>>> Is that still true, and do we even use string instructions still on
>>> those old CPUs?  Jan's fixes don't introduce any additional delays in
>>> the non-string-instruction paths.
>> 
>> Yes various of the CPUs with bugs used string instructions.
>> 
> 
> Which CPUs are you talking about here?

Indeed, I went through various specification updates/revision guides
without finding any relevant erratum.

>> 
>> Both string and non string instructions are used on modern CPUs,
>> so making any of that slower is not a good idea.
>> 
> 
> Obviously not, but I'm perfectly fine turning REP_GOOD off on old broken
> CPUs.

Yes, that's what my plan would have been too, had I been able to
identify any specific CPU(s) that actually suffer from such problems.

Jan

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

On Thu, Jan 05, 2012 at 06:03:23PM -0800, H. Peter Anvin wrote:
> On 01/05/2012 05:47 PM, Andi Kleen wrote:
> >>
> >> Is that still true, and do we even use string instructions still on
> >> those old CPUs?  Jan's fixes don't introduce any additional delays in
> >> the non-string-instruction paths.
> > 
> > Yes various of the CPUs with bugs used string instructions.
> > 
> 
> Which CPUs are you talking about here?

This was various iterations of K8 and PSC. Early Meroms may also have
had issues (not 100% sure).

> 
> > 
> > Both string and non string instructions are used on modern CPUs,
> > so making any of that slower is not a good idea.
> > 
> 
> Obviously not, but I'm perfectly fine turning REP_GOOD off on old broken
> CPUs.

That would be even worse.

You would slow a critical fast path operation down for something
that never happens?!?  There were big differences between strings
and the unrolled loop for several.

IMHO it's a war games situation: "the only way to win is not to play"

-Andi

Re: x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

>>> On 06.01.12 at 10:49, Andi Kleen <andi@firstfloor.org> wrote:
> On Thu, Jan 05, 2012 at 06:03:23PM -0800, H. Peter Anvin wrote:
>> On 01/05/2012 05:47 PM, Andi Kleen wrote:
>> >>
>> >> Is that still true, and do we even use string instructions still on
>> >> those old CPUs?  Jan's fixes don't introduce any additional delays in
>> >> the non-string-instruction paths.
>> > 
>> > Yes various of the CPUs with bugs used string instructions.
>> > 
>> 
>> Which CPUs are you talking about here?
> 
> This was various iterations of K8 and PSC. Early Meroms may also have
> had issues (not 100% sure).

I checked the most recent K8 (Hammer) and Pentium4 documentation,
and didn't find mention of anything related. So pointing out where you
found something relevant would be helpful. (And yes, as indicated in
my original mail I too seem to recall there being problems in this area,
but upon closer checking yesterday they all turned out unrelated to
this code.)

>> > Both string and non string instructions are used on modern CPUs,
>> > so making any of that slower is not a good idea.
>> > 
>> 
>> Obviously not, but I'm perfectly fine turning REP_GOOD off on old broken
>> CPUs.
> 
> That would be even worse.

How could you allow a CPU fall under REP_GOOD is its rep
implementation is buggy.

> You would slow a critical fast path operation down for something
> that never happens?!?

It does happen, just (so far) not in-tree. It's a latent problem that's
just waiting for someone else to run into. Apart from large bootmem
allocations (where not even the latency of the memory clearing
matters in any way, there's nothing I know of that would prevent
someone from vmalloc()-ing a huge block of memory and then
calling memset() on it (which ought to be tolerable in a preemptable
kernel at least).

Jan

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

> I checked the most recent K8 (Hammer) and Pentium4 documentation,
> and didn't find mention of anything related. So pointing out where you

Sorry don't really plan to do any errata archaeology for this folly.

Maybe it got fixed in the latest versions, I remember them being there
on some early ones at least.

> >> > so making any of that slower is not a good idea.
> >> > 
> >> 
> >> Obviously not, but I'm perfectly fine turning REP_GOOD off on old broken
> >> CPUs.
> > 
> > That would be even worse.
> 
> How could you allow a CPU fall under REP_GOOD is its rep
> implementation is buggy.

The kernel never does >4GB copies, memsets, so it's not buggy.
It's just like any other obscure errata that doesn't matter because
it's not exercised.

REP_GOOD just means "string instructions are faster for 4K operations"

> 
> > You would slow a critical fast path operation down for something
> > that never happens?!?
> 
> It does happen, just (so far) not in-tree. It's a latent problem that's
> just waiting for someone else to run into. Apart from large bootmem
> allocations (where not even the latency of the memory clearing

Modern kernels are bootmem less.

> matters in any way, there's nothing I know of that would prevent
> someone from vmalloc()-ing a huge block of memory and then
> calling memset() on it (which ought to be tolerable in a preemptable
> kernel at least).

I bet there are more "int"s in the allocators and other code. It would
surprise me if it worked even with a changed memset.

Besides it obviously would be a bad idea on non preemptive kernels,
and since that can always happen it would be bad code, hopefully
never hitting any trees.

If you need it in Xen please just add the cmps there.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

>>> On 06.01.12 at 11:37, Andi Kleen <andi@firstfloor.org> wrote:
>> > You would slow a critical fast path operation down for something
>> > that never happens?!?
>> 
>> It does happen, just (so far) not in-tree. It's a latent problem that's
>> just waiting for someone else to run into. Apart from large bootmem
>> allocations (where not even the latency of the memory clearing
> 
> Modern kernels are bootmem less.

It's not the traditional bootmem implementation anymore, but
alloc_bootmem() et al still exist, and still clear the allocated memory
(in __alloc_memory_core_early()). So there is a code path that can
validly be used (and it is this code path that is presenting one of the
problems with the non-pv-ops Xen kernels, as they're using flatmem
rather than sparsemem since their physical address space is always
fully continuous).

Jan

Re: x86-64: memset()/memcpy() not fully standards compliant

[Ingo Molnar]

* Andi Kleen <andi@firstfloor.org> wrote:

> You would slow a critical fast path operation down for 
> something that never happens?!?  There were big differences 
> between strings and the unrolled loop for several.

AFAICS Jan's approach speeds up memset(), so what you say is 
100% pure nonsense.

Thanks,

	Ingo

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

> It's not the traditional bootmem implementation anymore, but
> alloc_bootmem() et al still exist, and still clear the allocated memory
> (in __alloc_memory_core_early()). So there is a code path that can
> validly be used (and it is this code path that is presenting one of the
> problems with the non-pv-ops Xen kernels, as they're using flatmem
> rather than sparsemem since their physical address space is always
> fully continuous).

Yes but there should be no callers that do alloc_bootmem(4G)
The biggest ones afak are the 1GB pages I added some time ago.

How is it a problem in the non pv Xen kernels?

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

>>> On 06.01.12 at 12:08, Andi Kleen <andi@firstfloor.org> wrote:
>>  It's not the traditional bootmem implementation anymore, but
>> alloc_bootmem() et al still exist, and still clear the allocated memory
>> (in __alloc_memory_core_early()). So there is a code path that can
>> validly be used (and it is this code path that is presenting one of the
>> problems with the non-pv-ops Xen kernels, as they're using flatmem
>> rather than sparsemem since their physical address space is always
>> fully continuous).
> 
> Yes but there should be no callers that do alloc_bootmem(4G)
> The biggest ones afak are the 1GB pages I added some time ago.
> 
> How is it a problem in the non pv Xen kernels?

pgdat->node_mem_map[] exceeds 4G when total memory goes far
enough beyond 292G. At some later point ->node_page_cgroup[]
also exceeds 4G. Finally, the phys-to-machine mapping (which gets
resized during boot) exceeds 4G when crossing the 2T boundary.

Jan

Re: x86-64: memset()/memcpy() not fully standards compliant

[Andi Kleen]

> enough beyond 292G. At some later point ->node_page_cgroup[]
> also exceeds 4G. Finally, the phys-to-machine mapping (which gets
> resized during boot) exceeds 4G when crossing the 2T boundary.

Ok it's moot then because the old systems with problem didn't have
that much memory. So you could do without an errata workaround
and just extend the moves at the minor cost of a few prefixes
(I think that is what you did in your patch)

Only hope there are no new chips with similar issues...

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: x86-64: memset()/memcpy() not fully standards compliant

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

On Thu, 05 Jan 2012 14:04:30 GMT, Jan Beulich said:
> Forever these two functions have been limited to deal with at most 4G
> at a time. While I cannot point out an in-tree user that would require
> larger sizes, it is now the second time that within our Xen kernel we got
> bitten by that limitation.

Wow.  Second time?  What the heck is the Xen kernel *doing* that it's
trying to do over 4G in a single memset/memcpy?

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: x86-64: memset()/memcpy() not fully standards compliant

[H. Peter Anvin]

On 01/06/2012 11:44 AM, Andi Kleen wrote:
>> enough beyond 292G. At some later point ->node_page_cgroup[]
>> also exceeds 4G. Finally, the phys-to-machine mapping (which gets
>> resized during boot) exceeds 4G when crossing the 2T boundary.
> 
> Ok it's moot then because the old systems with problem didn't have
> that much memory. So you could do without an errata workaround
> and just extend the moves at the minor cost of a few prefixes
> (I think that is what you did in your patch)
> 

That seems like the way to go unless we can identify a real problem.

	-hpa

Re: x86-64: memset()/memcpy() not fully standards compliant

[Jan Beulich]

>>> On 06.01.12 at 21:40, <Valdis.Kletnieks@vt.edu> wrote:
> On Thu, 05 Jan 2012 14:04:30 GMT, Jan Beulich said:
>> Forever these two functions have been limited to deal with at most 4G
>> at a time. While I cannot point out an in-tree user that would require
>> larger sizes, it is now the second time that within our Xen kernel we got
>> bitten by that limitation.
> 
> Wow.  Second time?  What the heck is the Xen kernel *doing* that it's
> trying to do over 4G in a single memset/memcpy?

Please see my other reply to Andi for what data structures are
affected here (and starting at what total memory sizes).

Jan