Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Arjan van de Ven]

On Fri, 2006-06-23 at 15:01 +0000, Linux Kernel Mailing List wrote:
> commit c22ce143d15eb288543fe9873e1c5ac1c01b69a1
> tree dc7d457b8952fc50dfc90df659b35de4117c61fc
> parent 7dbdf43cfa635ddc3701cc8d1eab07597cd731c0
> author Hiro Yoshioka <hyoshiok@miraclelinux.com> Fri, 23 Jun 2006 16:04:16 -0700
> committer Linus Torvalds <torvalds@g5.osdl.org> Fri, 23 Jun 2006 21:42:56 -0700
> 
> [PATCH] x86: cache pollution aware __copy_from_user_ll()
> 
> Use the x86 cache-bypassing copy instructions for copy_from_user().
> 
> Some performance data are
> 
> Total of GLOBAL_POWER_EVENTS (CPU cycle samples)
> 
> 2.6.12.4.orig    1921587
> 2.6.12.4.nt      1599424
> 1599424/1921587=83.23% (16.77% reduction)

Hi,

while this patch will reduce the number of cycles spent in the kernel,
it's just pushing the cache miss to userspace (by virtue of doing a
cache flush effectively)... is this really the right thing? The total
memory bandwidth will actually increase with this patch if you're
unlucky (eg if userspace decides to write to this memory eventually)....

Greetings,
   Arjan van de Ven

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Dave Jones]

On Sat, Jun 24, 2006 at 04:42:32PM +0200, Arjan van de Ven wrote:
 > On Fri, 2006-06-23 at 15:01 +0000, Linux Kernel Mailing List wrote:
 > > commit c22ce143d15eb288543fe9873e1c5ac1c01b69a1
 > > tree dc7d457b8952fc50dfc90df659b35de4117c61fc
 > > parent 7dbdf43cfa635ddc3701cc8d1eab07597cd731c0
 > > author Hiro Yoshioka <hyoshiok@miraclelinux.com> Fri, 23 Jun 2006 16:04:16 -0700
 > > committer Linus Torvalds <torvalds@g5.osdl.org> Fri, 23 Jun 2006 21:42:56 -0700
 > > 
 > > [PATCH] x86: cache pollution aware __copy_from_user_ll()
 > > 
 > > Use the x86 cache-bypassing copy instructions for copy_from_user().
 > > 
 > > Some performance data are
 > > 
 > > Total of GLOBAL_POWER_EVENTS (CPU cycle samples)
 > > 
 > > 2.6.12.4.orig    1921587
 > > 2.6.12.4.nt      1599424
 > > 1599424/1921587=83.23% (16.77% reduction)
 > 
 > Hi,
 > 
 > while this patch will reduce the number of cycles spent in the kernel,
 > it's just pushing the cache miss to userspace (by virtue of doing a
 > cache flush effectively)... is this really the right thing? The total
 > memory bandwidth will actually increase with this patch if you're
 > unlucky (eg if userspace decides to write to this memory eventually)....

But this is a copy _from_ userspace. The userspace app has likely already
brought it into cache before we do the __copy_from_user_ll

		Dave

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

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Linus Torvalds]

On Sat, 24 Jun 2006, Arjan van de Ven wrote:
> 
> while this patch will reduce the number of cycles spent in the kernel,
> it's just pushing the cache miss to userspace (by virtue of doing a
> cache flush effectively)... is this really the right thing? The total
> memory bandwidth will actually increase with this patch if you're
> unlucky (eg if userspace decides to write to this memory eventually)....

No. It's for copying _from_ user space, ie a "write()" system call. So 
what it does is to effectively try to use non-temporal stores to the page 
cache - since the page cache is usually not read directly afterwards (at 
least not soon enough for L1 caches to help).

I don't generally like cache tricks either (caches tend to be better than 
humans, or at least get there fairly soon), but this one does seem very 
valid.

		Linus

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Arjan van de Ven]

On Sat, 2006-06-24 at 12:21 -0700, Linus Torvalds wrote:
> 
> On Sat, 24 Jun 2006, Arjan van de Ven wrote:
> > 
> > while this patch will reduce the number of cycles spent in the kernel,
> > it's just pushing the cache miss to userspace (by virtue of doing a
> > cache flush effectively)... is this really the right thing? The total
> > memory bandwidth will actually increase with this patch if you're
> > unlucky (eg if userspace decides to write to this memory eventually)....
> 
> No. It's for copying _from_ user space, ie a "write()" system call.

eh DOH

never mind

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Andi Kleen]

Linus Torvalds <torvalds@osdl.org> writes:

> On Sat, 24 Jun 2006, Arjan van de Ven wrote:
> > 
> > while this patch will reduce the number of cycles spent in the kernel,
> > it's just pushing the cache miss to userspace (by virtue of doing a
> > cache flush effectively)... is this really the right thing? The total
> > memory bandwidth will actually increase with this patch if you're
> > unlucky (eg if userspace decides to write to this memory eventually)....
> 
> No. It's for copying _from_ user space, ie a "write()" system call. So 
> what it does is to effectively try to use non-temporal stores to the page 
> cache - since the page cache is usually not read directly afterwards (at 
> least not soon enough for L1 caches to help).
> 
> I don't generally like cache tricks either (caches tend to be better than 
> humans, or at least get there fairly soon), but this one does seem very 
> valid.

Problem is that it will likely wreck pipe and AF_UNIX (= X server) 
performance. These read the buffer quickly again.

If you do something like this it would be better to define
a special function and use it for real FS traffic only.

-Andi

Re: [PATCH] x86: cache pollution aware __copy_from_user_ll()

[Linus Torvalds]

On Mon, 26 Jun 2006, Andi Kleen wrote:
> 
> Problem is that it will likely wreck pipe and AF_UNIX (= X server) 
> performance. These read the buffer quickly again.

Look at the patch, dammit!

> If you do something like this it would be better to define
> a special function and use it for real FS traffic only.

Has anybody taken even a five-second look at the patch itself, or are you 
all just complaining without knowing what you're complaining about?

		Linus