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=-8.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 2A45CC433ED for ; Sat, 10 Apr 2021 14:19:12 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A7BD4610CD for ; Sat, 10 Apr 2021 14:19:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A7BD4610CD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=ACULAB.COM Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:In-Reply-To:References:Message-ID:Date: Subject:CC:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=b5OariQC0yf6jBBU/rH8WKAOW/HVLTA9CoARbA31htU=; b=Sd0+RCLVpMyvMFMLoP0nufVAS vkZLVVEAJw6ldUeLHT+CDkkWFxy0MNVEQco54+pNgejyTPs8Z3Dlo4nvX1Lp2B/leRyQbR1HsyrWz 84rolOlHJeH7vbKFeUBveNHulc0/Xw6uM2j496W0HFemXsw8XeWWpTVfQDrk43/ZUDMNl7dnsDVMR 22jBQivci9eF9E1dzy/ViRMM34xfZkIql/EWQBSIwdz9dxor2EqSH5CUyDg/Onivg/ufNjhv6FWNV PbW1qS1fl5Vx9qqW4Ueodv5PeZlFfBG7QsFZcu9d4Ix41U8U8RVApbnjNuKp71Tj1dDXB5sKuONjS z12KRWwNg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lVEQJ-002wxu-OS; Sat, 10 Apr 2021 14:17:19 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lVEQF-002wxk-RH for linux-arm-kernel@desiato.infradead.org; Sat, 10 Apr 2021 14:17:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:MIME-Version:In-Reply-To:References:Message-ID:Date:Subject:CC: To:From:Sender:Reply-To:Content-ID:Content-Description; bh=aMyKgyy/K7XZrmJf8Cwzzurugif2dtZmgpas4lx4sow=; b=lURiDC8+i1c5yWUsc152fEI1W2 5uVfcm4Xm+nvjjMbFr7TJZbKWZ3jB1tJ9Vwbe9eDRT+2LgX9xw3UffgcVcKzzCK2JR5nX5gMMa+za Zw+wh7Ml4bhtm9AF/Py2NllLXVMn59REQLYbLsd6kMlE/RDOVVWooDLRV6y0wGb5+EU9YilB+3U+K zcqmpsmFe093pXjDWtnTHo/Rh8Ze0MQJsO0Q8/nhPwgLpAApM4ygH3Uz5v/1/XFkFLZ6GujB7ptH7 Xnn7n6E5Ng2TVF9RPu4qX3yCWJif+/hcDc5tdAh7JrdZyZJtN7YHp2mm1ipaoDExNsdrBcgDOda5d mAuM9gYA==; Received: from eu-smtp-delivery-151.mimecast.com ([185.58.85.151]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lVEQ9-0058We-P6 for linux-arm-kernel@lists.infradead.org; Sat, 10 Apr 2021 14:17:14 +0000 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 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210410_071710_107552_A0C699BD X-CRM114-Status: GOOD ( 25.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 requirement > '__builtin_offsetof(struct page, lru) == __builtin_offsetof(struct folio, lru)' "offsetof(struct page, > lru) == offsetof(struct folio, lru)" > > FOLIO_MATCH(lru, lru); > > include/linux/mm_types.h:272:2: note: expanded from macro 'FOLIO_MATCH' > > static_assert(offsetof(struct page, pg) == offsetof(struct folio, fl)) > > Well, this is interesting. pahole reports: > > 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 */ > > so this assert has absolutely done its job. > > 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. > > 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. > > We can fix it like this ... > > +++ 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 on > * 32-bit architectures. > */ > - dma_addr_t dma_addr; > + dma_addr_t dma_addr __packed; > }; > struct { /* slab, slob and slub */ > union { > > 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...) > > We could also do: > > + dma_addr_t dma_addr __packed __aligned(sizeof(void *)); I wonder if __aligned(n) should be defined as __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.) > > and I see pahole, at least sees this correctly: > > 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. > > 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). David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel