mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Vineet Gupta' <vineet.gupta1@synopsys.com>,
	Alexey Brodkin <alexey.brodkin@synopsys.com>,
	"linux-snps-arc@lists.infradead.org" 
	<linux-snps-arc@lists.infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH] ARC: Explicitly set ARCH_SLAB_MINALIGN = 8
Date: Tue, 12 Feb 2019 17:30:38 +0000	[thread overview]
Message-ID: <f83412d1e7fd4d67b4632c1f81fae963@AcuMS.aculab.com> (raw)
In-Reply-To: <81017fe4-b31f-4942-e822-a7b70008b74d@synopsys.com>

From:  Vineet Gupta
> Sent: 12 February 2019 17:17
> 
> On 2/8/19 2:55 AM, Alexey Brodkin wrote:
> > By default ARCH_SLAB_MINALIGN is defined in "include/linux/slab.h" as
> > "__alignof__(unsigned long long)" which looks fine but not for ARC.
> 
> Just for the record, the issue happens because a LLOCKD (exclusive 64-bit load)
> was trying to use a 32-bit aligned effective address (for atomic64_t), not allowed
> by ISA (LLOCKD can only take 64-bit aligned address, even when the CPU has
> unaligned access enabled).
> 
> This in turn was happening because this word is embedded in some other struct and
> happens to be 4 byte aligned
> 
> 
> > ARC tools ABI sets align of "long long" the same as for "long" = 4
> > instead of 8 one may think of.

Right, but __alignof__() doesn't have to return the alignment that would
be used for a data item of the specified type.
(Read the gcc 'bug' info for gory details.)

On i386 __alignof__(long long) is 8, but structure members of type 'long long'
are 4 byte aligned and the alignment of a structure with a 'long long' member
is only 4.
(Although the microsoft compiler returns 4.)

> Right, this was indeed unexpected and not like most other arches. ARCv2 ISA allows
> regular 64-bit loads/stores (LDD/STD) to take 32-bit aligned addresses. Thus ABI
> relaxing the alignment for 64-bit data potentially causes more packing and less
> space waste. But on the flip side we need to waste space at arbitrary places liek
> this.
> 
> So this is all good and theory, but I'm not 100% sure how slab alignment helps
> here (and is future proof). So the outer struct with embedded atomic64_t was
> allocated via slab and your patch ensures that outer struct is 64-bit aligned ?

Presumable 'atomic64_t' has an alignment attribute to force 8 byte alignment.

> But how does that guarantee that all embedded atomic64_t in there will be 64-bit
> aligned (in future say) in the light of ARC ABI and the gcc bug/feature which
> Peter alluded to
> 
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10360
> 
> > Thus slab allocator may easily allocate a buffer which is 32-bit aligned.
> > And most of the time it's OK until we start dealing with 64-bit atomics
> > with special LLOCKD/SCONDD instructions which (as opposed to their 32-bit
> > counterparts LLOCK/SCOND) operate with full 64-bit words but those words
> > must be 64-bit aligned.
> 
> Some of this text needed to go above to give more context.

I suspect the slab allocator should be returning 8 byte aligned addresses
on all systems....

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  reply	other threads:[~2019-02-12 17:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-08 10:55 Alexey Brodkin
2019-02-12 17:17 ` Vineet Gupta
2019-02-12 17:30   ` David Laight [this message]
2019-02-12 17:45     ` Vineet Gupta
2019-02-13 12:56       ` Peter Zijlstra
2019-02-13 14:13         ` David Laight
2019-02-13 23:23         ` Vineet Gupta
2019-02-14  8:50           ` Alexey Brodkin
2019-02-14 10:28             ` Peter Zijlstra
2019-02-15  1:34             ` Vineet Gupta
2019-02-18  8:53               ` Alexey Brodkin
2019-02-19 23:30                 ` Vineet Gupta
2019-02-14 10:31           ` Peter Zijlstra
2019-02-14 10:44             ` Alexey Brodkin
2019-02-14 11:08               ` Peter Zijlstra
2019-02-14 12:05                 ` Alexey Brodkin
2019-02-14 12:24                   ` Peter Zijlstra
2019-02-14 14:14                 ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f83412d1e7fd4d67b4632c1f81fae963@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=alexey.brodkin@synopsys.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=vineet.gupta1@synopsys.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome