From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, PDS_BAD_THREAD_QP_64,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D93AC433ED for ; Sat, 10 Apr 2021 14:17:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 16AAC610D1 for ; Sat, 10 Apr 2021 14:17:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 16AAC610D1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=ACULAB.COM Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7B03C6B006C; Sat, 10 Apr 2021 10:17:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 75FC06B006E; Sat, 10 Apr 2021 10:17:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5D9338E0001; Sat, 10 Apr 2021 10:17:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0080.hostedemail.com [216.40.44.80]) by kanga.kvack.org (Postfix) with ESMTP id 40F9F6B006C for ; Sat, 10 Apr 2021 10:17:10 -0400 (EDT) Received: from smtpin37.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id E5F3F62C0 for ; Sat, 10 Apr 2021 14:17:09 +0000 (UTC) X-FDA: 78016659378.37.D3ED481 Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.85.151]) by imf13.hostedemail.com (Postfix) with ESMTP id B71D5E000119 for ; Sat, 10 Apr 2021 14:17:06 +0000 (UTC) Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) (Using TLS) by relay.mimecast.com with ESMTP id uk-mta-84-kCKo1-18OZuYsleNdk-yPA-1; Sat, 10 Apr 2021 15:17:06 +0100 X-MC-Unique: kCKo1-18OZuYsleNdk-yPA-1 Received: from AcuMS.Aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) by AcuMS.aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 10 Apr 2021 15:17:05 +0100 Received: from AcuMS.Aculab.com ([fe80::994c:f5c2:35d6:9b65]) by AcuMS.aculab.com ([fe80::994c:f5c2:35d6:9b65%12]) with mapi id 15.00.1497.012; Sat, 10 Apr 2021 15:17:05 +0100 From: David Laight To: 'Matthew Wilcox' , kernel test robot CC: "linux-mm@kvack.org" , "kbuild-all@lists.01.org" , "clang-built-linux@googlegroups.com" , "linux-kernel@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , "linuxppc-dev@lists.ozlabs.org" , "linux-arm-kernel@lists.infradead.org" , Jesper Dangaard Brouer , "David S. Miller" Subject: RE: Bogus struct page layout on 32-bit Thread-Topic: Bogus struct page layout on 32-bit Thread-Index: AQHXLbNopyewSd0HZEaZbqB5fscTXqqtw2Cg Date: Sat, 10 Apr 2021 14:17:04 +0000 Message-ID: References: <20210409185105.188284-3-willy@infradead.org> <202104100656.N7EVvkNZ-lkp@intel.com> <20210410024313.GX2531743@casper.infradead.org> In-Reply-To: <20210410024313.GX2531743@casper.infradead.org> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=C51A453 smtp.mailfrom=david.laight@aculab.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Stat-Signature: ekc6fgphnqyx3qfokxs937ry9q641crd X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B71D5E000119 Received-SPF: none (aculab.com>: No applicable sender policy available) receiver=imf13; identity=mailfrom; envelope-from=""; helo=eu-smtp-delivery-151.mimecast.com; client-ip=185.58.85.151 X-HE-DKIM-Result: none/none X-HE-Tag: 1618064226-42734 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: Matthew Wilcox > Sent: 10 April 2021 03:43 > On Sat, Apr 10, 2021 at 06:45:35AM +0800, kernel test robot wrote: > > >> include/linux/mm_types.h:274:1: error: static_assert failed due to r= equirement > '__builtin_offsetof(struct page, lru) =3D=3D __builtin_offsetof(struct fo= lio, lru)' "offsetof(struct page, > lru) =3D=3D offsetof(struct folio, lru)" > > FOLIO_MATCH(lru, lru); > > include/linux/mm_types.h:272:2: note: expanded from macro 'FOLIO_MAT= CH' > > static_assert(offsetof(struct page, pg) =3D=3D offsetof(stru= ct folio, fl)) >=20 > Well, this is interesting. pahole reports: >=20 > struct page { > long unsigned int flags; /* 0 4 *= / > /* XXX 4 bytes hole, try to pack */ > union { > struct { > struct list_head lru; /* 8 8 *= / > ... > struct folio { > union { > struct { > long unsigned int flags; /* 0 4 *= / > struct list_head lru; /* 4 8 *= / >=20 > so this assert has absolutely done its job. >=20 > But why has this assert triggered? Why is struct page layout not what > we thought it was? Turns out it's the dma_addr added in 2019 by commit > c25fff7171be ("mm: add dma_addr_t to struct page"). On this particular > config, it's 64-bit, and ppc32 requires alignment to 64-bit. So > the whole union gets moved out by 4 bytes. >=20 > Unfortunately, we can't just fix this by putting an 'unsigned long pad' > in front of it. It still aligns the entire union to 8 bytes, and then > it skips another 4 bytes after the pad. >=20 > We can fix it like this ... >=20 > +++ b/include/linux/mm_types.h > @@ -96,11 +96,12 @@ struct page { > unsigned long private; > }; > struct { /* page_pool used by netstack */ > + unsigned long _page_pool_pad; > /** > * @dma_addr: might require a 64-bit value even o= n > * 32-bit architectures. > */ > - dma_addr_t dma_addr; > + dma_addr_t dma_addr __packed; > }; > struct { /* slab, slob and slub */ > union { >=20 > but I don't know if GCC is smart enough to realise that dma_addr is now > on an 8 byte boundary and it can use a normal instruction to access it, > or whether it'll do something daft like use byte loads to access it. I'm betting it will use byte accesses. Checked - it does seem to use 4-byte accesses. (godbolt is rather short of 32 bit compilers...) >=20 > We could also do: >=20 > + dma_addr_t dma_addr __packed __aligned(sizeof(voi= d *)); I wonder if __aligned(n) should be defined as =09__attribute__((packed,aligned(n)) I don't think you ever want the 'unpacked' variant. But explicitly reducing the alignment of single member is much better than the habit of marking the structure 'packed'. (Never mind the habit of adding __packed 'because we don't want the compiler to add random padding.) >=20 > and I see pahole, at least sees this correctly: >=20 > struct { > long unsigned int _page_pool_pad; /* 4 4 = */ > dma_addr_t dma_addr __attribute__((__aligned__(4)= )); /* 8 8 */ > } __attribute__((__packed__)) __attribute__((__aligned__(= 4))); Is the attribute on the struct an artifact of pahole? It should just have an alignment of 4 without anything special. >=20 > This presumably affects any 32-bit architecture with a 64-bit phys_addr_t > / dma_addr_t. Advice, please? Only those where a 64-bit value is 64-bit aligned. So that excludes x86 (which can have 64-bit dma) but includes sparc32 (which probably doesn't). =09David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)