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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3060CC433EF for ; Tue, 3 May 2022 16:24:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239519AbiECQ2B convert rfc822-to-8bit (ORCPT ); Tue, 3 May 2022 12:28:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239526AbiECQ2A (ORCPT ); Tue, 3 May 2022 12:28:00 -0400 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 949583CFFD for ; Tue, 3 May 2022 09:24:26 -0700 (PDT) Received: from fraeml713-chm.china.huawei.com (unknown [172.18.147.207]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Kt4w01dPMz67bVT; Wed, 4 May 2022 00:21:20 +0800 (CST) Received: from lhreml710-chm.china.huawei.com (10.201.108.61) by fraeml713-chm.china.huawei.com (10.206.15.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 3 May 2022 18:24:23 +0200 Received: from localhost (10.202.226.42) by lhreml710-chm.china.huawei.com (10.201.108.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 3 May 2022 17:24:22 +0100 Date: Tue, 3 May 2022 17:24:21 +0100 From: Jonathan Cameron To: Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= CC: Jonathan Cameron , , "Akinobu Mita" , Alexandru Lazar , Alexandru Tachici , Antoniu Miclaus , Charles-Antoine Couret , Cosmin Tanislav , Cristian Pop , "David Lechner" , Ivan Mikhaylov , Jacopo Mondi , Jean-Baptiste Maneyrol , Lars-Peter Clausen , "Marcelo Schmitt" , =?ISO-8859-1?Q?M=E5rten?= Lindahl , Matt Ranostay , Michael Hennerich , Michael Welling , "Mugilraj Dhavachelvan" , Navin Sankar Velliangiri , Nuno =?ISO-8859-1?Q?S=E1?= , "Paul Cercueil" , Phil Reid , Puranjay Mohan , Ricardo Ribalda , Robert Jones , Rui Miguel Silva , Sean Nyekjaer , Tomas Melin , Tomislav Denis Subject: Re: [PATCH 01/92] iio: core: Fix IIO_ALIGN as it was not sufficiently large on some platforms. Message-ID: <20220503172421.0000615c@Huawei.com> In-Reply-To: <20220503142725.h6pcf2socuxgteix@pengutronix.de> References: <20220503085935.1533814-1-jic23@kernel.org> <20220503085935.1533814-2-jic23@kernel.org> <20220503142725.h6pcf2socuxgteix@pengutronix.de> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.29; i686-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.202.226.42] X-ClientProxiedBy: lhreml723-chm.china.huawei.com (10.201.108.74) To lhreml710-chm.china.huawei.com (10.201.108.61) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org On Tue, 3 May 2022 16:27:25 +0200 Uwe Kleine-König wrote: > On Tue, May 03, 2022 at 09:58:04AM +0100, Jonathan Cameron wrote: > > From: Jonathan Cameron > > > > Discussion of the series: > > https://lore.kernel.org/all/20220405135758.774016-1-catalin.marinas@arm.com/ > > mm, arm64: Reduce ARCH_KMALLOC_MINALIGN brought to my attention that > > our current IIO usage of L1CACHE_ALIGN is insufficient as their are Arm > > platforms out their with non coherent DMA and larger cache lines at > > at higher levels of their cache hierarchy. > > > > Note this patch will greatly reduce the padding on some architectures > > that have smaller requirements for DMA safe buffers. > > > > The history of changing values of ARCH_KMALLOC_MINALIGN via > > ARCH_DMA_MINALIGN on arm64 is rather complex. I'm not tagging this > > as fixing a particular patch from that route as it's not clear what to tag. > > > > Most recently a change to bring them back inline was reverted because > > of some Qualcomm Kryo cores with an L2 cache with 128-byte lines > > sitting above the point of coherency. > > > > c1132702c71f Revert "arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES)" > > That reverts: > > 65688d2a05de arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES) which > > refers to the change originally being motivated by Thunder x1 performance > > rather than correctness. > > > > Fixes: 6f7c8ee585e9d ("staging:iio: Add ability to allocate private data space to iio_allocate_device") > > Signed-off-by: Jonathan Cameron > > --- > > include/linux/iio/iio.h | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > > index faf00f2c0be6..30937f8f9424 100644 > > --- a/include/linux/iio/iio.h > > +++ b/include/linux/iio/iio.h > > @@ -9,6 +9,7 @@ > > > > #include > > #include > > +#include > > #include > > #include > > /* IIO TODO LIST */ > > @@ -657,8 +658,13 @@ static inline void *iio_device_get_drvdata(const struct iio_dev *indio_dev) > > return dev_get_drvdata(&indio_dev->dev); > > } > > > > -/* Can we make this smaller? */ > > -#define IIO_ALIGN L1_CACHE_BYTES > > +/* > > + * Used to ensure the iio_priv() structure is aligned to allow that structure > > + * to in turn include IIO_ALIGN'd elements such as buffers which must not share > > + * cachelines with the rest of the structure, thus making them safe for use with > > + * non-coherent DMA. > > + */ > > +#define IIO_ALIGN ARCH_KMALLOC_MINALIGN > > Given the purpose of IIO_ALIGN is to define the alignment for DMA'able > memory, I wonder why it's called "IIO_ALIGN" and not for example > "DMA_MINALIGN". Much like crypto I want a single place that provides the IIO requirements for this. Could rename it IIO_DMA_MINALIGN I guess. The reason behind that is to allow for a switch on mass if a new approach is accepted along the lines of what Catalin proposed. The discussions around CRYPTO made it clear that there are sometimes additional requirements from a subsystem beyond simply that of DMA (IIO has a similar issue to crypto that mean it's not simple to shift the alignment requirements at runtime because the compiler is getting told things are aligned to a higher degree than the allocations). https://lore.kernel.org/all/20220405135758.774016-8-catalin.marinas@arm.com/ and below that point. > > There is nothing iio specific about this value, is there? Then > consequently it doesn't need to be defined in an iio header, but > somewhere generic. Or even one step further: Why isn't there a macro > __align_for_dma that can be used directly to annotate the relevant > member in a struct? There is, but it's not currently available on all architectures. ARCH_DMA_MINALIGN Catalin's series proposed making it generally available: https://lore.kernel.org/all/20220405135758.774016-2-catalin.marinas@arm.com/ but suggestion for now was to go with ARCH_KMALLOC_MINALIGN https://lore.kernel.org/linux-iio/Yl6jB5DOUy+Yqyzl@arm.com/ Thanks, Jonathan > > Best regards > Uwe >