2015-11-03 21:01:24

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On Sun, Oct 25, 2015 at 4:49 AM, Helge Deller <[email protected]> wrote:
>
> please pull some patches for the parisc architecture for kernel v4.3 from:

So no way was I going to pull that for 4.3, and I delayed it to the
merge window.

However, even now that we're in the merge window, and I look at it again:

> The most important change is that we reduce L1_CACHE_BYTES to 16 bytes, for
> which a trivial patch for XPS in the network layer was needed.

I'd really want the network people involved with that change, and I'm
also wondering why you seem to want to re-define L1_CACHE_BYTES to
something that it isn't.

I doubt the PA-RISC L1 cacheline really is 16 bytes. So this seems to
be more of a hack around the fact that some data structures may be
over-aligned, and using that L1_CACHE_BYTES for aligning things that
really don't want to be that aligned. Maybe it casues less sharing,
but if it does so at the cost of excessive memory use, it's still
wrong.

But that in turn says to me "We should fix the *real* problem, rather
than hack around it by having PA-RISC lie about its L1 cache size".

Is there any particular over-alignment that you have determined to be
the real problem?

Also, just looking at other things, we currently do have openrisc that has

#define L1_CACHE_BYTES 16

so presumably openrisc would have had an issue with that XPS thing,
and how XPS_MIN_MAP_ALLOC can go negative for a 16-byte case (well,
it's zero on 32-bit architectures).

David: the issue wrt XPS is this:

#define XPS_MIN_MAP_ALLOC ((L1_CACHE_BYTES - sizeof(struct xps_map)) \
/ sizeof(u16))

Comments?

Linus


2015-11-03 21:33:32

by David Miller

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

From: Linus Torvalds <[email protected]>
Date: Tue, 3 Nov 2015 13:01:21 -0800

> David: the issue wrt XPS is this:
>
> #define XPS_MIN_MAP_ALLOC ((L1_CACHE_BYTES - sizeof(struct xps_map)) \
> / sizeof(u16))
>
> Comments?

The PARISC folks did discuss this with us networking folks...

http://marc.info/?t=144554413000001&r=1&w=2

2015-11-03 22:07:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On Tue, Nov 3, 2015 at 1:33 PM, David Miller <[email protected]> wrote:
>
>> David: the issue wrt XPS is this:
>>
>> #define XPS_MIN_MAP_ALLOC ((L1_CACHE_BYTES - sizeof(struct xps_map)) \
>> / sizeof(u16))
>>
>> Comments?
>
> The PARISC folks did discuss this with us networking folks...
>
> http://marc.info/?t=144554413000001&r=1&w=2

Ok. So that still leaves the "is it sane to lie about your
L1_CACHE_BYTES" question...

Linus

2015-11-03 23:04:01

by Helge Deller

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

Hi Linus,

On 03.11.2015 22:01, Linus Torvalds wrote:
> On Sun, Oct 25, 2015 at 4:49 AM, Helge Deller <[email protected]> wrote:
>>
>> please pull some patches for the parisc architecture for kernel v4.3 from:
>
> So no way was I going to pull that for 4.3,

Yes, since you didn't pulled I assumed you saw some kind of problem with the patches.
Maybe it's even my fault, because I should have explained some more in the pull request,
e.g. that all patches were discussed with the various stakeholders, and e.g. that
I was late in sending this pull request, because I was waiting for some benchmark results.

> and I delayed it to the merge window.

Ok.

> However, even now that we're in the merge window, and I look at it again:
>
>> The most important change is that we reduce L1_CACHE_BYTES to 16 bytes, for
>> which a trivial patch for XPS in the network layer was needed.
>
> I'd really want the network people involved with that change,

As David already answered, it was discussed with them:
http://marc.info/?t=144554413000001&r=1&w=2

> and I'm
> also wondering why you seem to want to re-define L1_CACHE_BYTES to
> something that it isn't.
> I doubt the PA-RISC L1 cacheline really is 16 bytes.

Sadly it's nowhere clearly documented how big the L1 cacheline of parisc really is.

We are currently experimenting a lot with improving spinlocks on hppa,
that's why we play around with the L1 cache size setting.

In one of the mail threads (where I actually wanted to align the hashes
which we need to protect/simulate the atomic locks) James Bottomleys
gave a pretty good explanation of why it might be beneficial to
modify L1_CACHE_BYTES for parisc:
http://permalink.gmane.org/gmane.linux.ports.parisc/26040
The whole mail thread is here:
http://thread.gmane.org/gmane.linux.ports.parisc/26000

> So this seems to
> be more of a hack around the fact that some data structures may be
> over-aligned, and using that L1_CACHE_BYTES for aligning things that
> really don't want to be that aligned. Maybe it casues less sharing,
> but if it does so at the cost of excessive memory use, it's still
> wrong.
>
> But that in turn says to me "We should fix the *real* problem, rather
> than hack around it by having PA-RISC lie about its L1 cache size".
>
> Is there any particular over-alignment that you have determined to be
> the real problem?

I was not very much concerned about any over-alignment, but about the
performance. Reducing L1_CACHE_BYTES gave a performance improvement
on parisc, most likely since we protect atomic accesses through our
atomic spinlocks anyway.

> Also, just looking at other things, we currently do have openrisc that has
>
> #define L1_CACHE_BYTES 16
>
> so presumably openrisc would have had an issue with that XPS thing,

and mn10300.

Helge

2015-11-03 23:25:18

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On Tue, Nov 3, 2015 at 3:03 PM, Helge Deller <[email protected]> wrote:
>
> Sadly it's nowhere clearly documented how big the L1 cacheline of parisc really is.

Wow.

Particularly that "it might actually be 16 bytes" from the thread
according to John David Anglin. I didn't expect anybody to really have
that small a line size any more.

> I was not very much concerned about any over-alignment, but about the
> performance. Reducing L1_CACHE_BYTES gave a performance improvement
> on parisc, most likely since we protect atomic accesses through our
> atomic spinlocks anyway.

Well, we do end up using L1_CACHE_BYTES to avoid false sharing in some
places, where it's not so much about atomic accesses, as just trying
to avoid having different CPU's step on each other when not needed. So
it's not necessarily about atomic accesses per se.

But if it's actually possible that the pa-risc L1 line size is really
just 16 bytes, I guess that objection to the patch goes away. My
automatic reaction was "that's not real, it's some odd workaround",
but if it is actually real...

Linus

2015-11-03 23:54:52

by John David Anglin

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On 2015-11-03, at 6:25 PM, Linus Torvalds wrote:

> But if it's actually possible that the pa-risc L1 line size is really
> just 16 bytes, I guess that objection to the patch goes away. My
> automatic reaction was "that's not real, it's some odd workaround",
> but if it is actually real...

See page 10 in this document:
https://parisc.wiki.kernel.org/images-parisc/e/e9/PA-8700wp.pdf

It shows the PA-8700 L1 design. James' comments and this paper are the
basis for this change.

Dave
--
John David Anglin [email protected]


2015-11-03 23:55:01

by Guy Harris

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3


On Nov 3, 2015, at 3:03 PM, Helge Deller <[email protected]> wrote:

> Sadly it's nowhere clearly documented how big the L1 cacheline of parisc really is.

To which particular PA-RISC processor are you referring? It might not be the same on all processors.

If openpa.net is to be believed, then:

The 7100LC has 32 byte cache lines on the off-chip cache:

http://www.openpa.net/pa-risc_processor_pa-7100lc.html

and the 8500 has "32 or 64 Byte cache line size", which may be referring to the on-chip caches:

http://www.openpa.net/pa-risc_processor_pa-8500.html-

2015-11-03 23:51:57

by Guy Harris

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3


On Nov 3, 2015, at 3:43 PM, Guy Harris <[email protected]> wrote:

> To which particular PA-RISC processor are you referring? It might not be the same on all processors.

Chapter 3 "Addressing and Access Control" of PA-RISC 2.0 Architecture:

http://h21007.www2.hp.com/portal/download/files/unprot/parisc20/PA_3_addressing.pdf

says

A consistent software view of cache operation requires that implementations never write a clean cache line back to memory. (A cache line can be 16, 32, or 64 bytes in length.) Clean means “not stored into” as opposed to “not changed”. Dirty means “stored into”. A cache line which was stored into in such a way that it was unchanged is considered to be dirty.

so, architecturally, it can be 16, 32, or 64 bytes.

I'm not sure what

When Linux says 'line size' it generally means the cache ownership line size: the minimum block the inter cpu coherence operates on. Most of the architectural evidence for PA systems suggests that this is 16

from the mail message cited means when it speaks of architectural evidence; is that line size different from the line size in the PA-RISC 2.0 Architecture manual? That line size presumably isn't the burst size:

128 seems to be the cache burst fill size (the number of bytes that will be pulled into the
cache by a usual operation touching any byte in the area).-

2015-11-03 23:53:36

by John David Anglin

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On 2015-11-03, at 6:43 PM, Guy Harris wrote:

>
> On Nov 3, 2015, at 3:03 PM, Helge Deller <[email protected]> wrote:
>
>> Sadly it's nowhere clearly documented how big the L1 cacheline of parisc really is.
>
> To which particular PA-RISC processor are you referring? It might not be the same on all processors.
>
> If openpa.net is to be believed, then:
>
> The 7100LC has 32 byte cache lines on the off-chip cache:
>
> http://www.openpa.net/pa-risc_processor_pa-7100lc.html
>
> and the 8500 has "32 or 64 Byte cache line size", which may be referring to the on-chip caches:
>
> http://www.openpa.net/pa-risc_processor_pa-8500.html

Yes, this is correct but these numbers relate to the memory interface. The PA8800 and PA8900 have 128 byte
memory interfaces. These numbers are reported by firmware PDC calls. This whole discussion started when I
suggested that we needed to bump L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 processors.

--
John David Anglin [email protected]


2015-11-06 14:10:30

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [GIT PULL] parisc architecture updates for v4.3

On Wed, Nov 4, 2015 at 12:03 AM, Helge Deller <[email protected]> wrote:
>> Also, just looking at other things, we currently do have openrisc that has
>>
>> #define L1_CACHE_BYTES 16
>>
>> so presumably openrisc would have had an issue with that XPS thing,
>
> and mn10300.

And several other architectures that define L1_CACHE_BYTES using
L1_CACHE_SHIFT.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds