From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D5BF133F7 for ; Fri, 21 Apr 2023 14:58:27 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C877D1480; Fri, 21 Apr 2023 07:59:10 -0700 (PDT) Received: from [10.57.23.51] (unknown [10.57.23.51]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8010A3F6C4; Fri, 21 Apr 2023 07:58:24 -0700 (PDT) Message-ID: <4bd8ce51-5874-0aa3-bc82-fec0cee9b8f1@arm.com> Date: Fri, 21 Apr 2023 15:58:18 +0100 Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [RFC v1 3/4] swiotlb: Allow dynamic allocation of bounce buffers Content-Language: en-GB To: =?UTF-8?B?UGV0ciBUZXNhxZnDrWs=?= , Christoph Hellwig Cc: Petr Tesarik , Jonathan Corbet , Marek Szyprowski , Borislav Petkov , "Paul E. McKenney" , Andrew Morton , Randy Dunlap , Damien Le Moal , Kim Phillips , "Steven Rostedt (Google)" , "open list:DOCUMENTATION" , open list , "open list:DMA MAPPING HELPERS" , Roberto Sassu , Alexander Graf References: <0334a54332ab75312c9de825548b616439dcc9f5.1679309810.git.petr.tesarik.ext@huawei.com> <20230328040724.GB25506@lst.de> <4268fa4e-4f0f-a2f6-a2a5-5b78ca4a073d@huaweicloud.com> <8cf7c515-9ce6-a2ed-0643-972aa3eba2fb@huaweicloud.com> <20230407055704.GD6803@lst.de> <20230407121555.4290a011@meshulam.tesarici.cz> <20230421150349.35966e0b@meshulam.tesarici.cz> From: Robin Murphy In-Reply-To: <20230421150349.35966e0b@meshulam.tesarici.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2023-04-21 14:03, Petr Tesařík wrote: > Hi Christoph! > > I'd like to follow up on this sub-thread: > > On Fri, 7 Apr 2023 12:15:55 +0200 > Petr Tesařík wroe: > >> On Fri, 7 Apr 2023 07:57:04 +0200 >> Christoph Hellwig wrote: >> [...] >>> (Btw, in case anyone is interested, we really need to get started >>> on moving the dma fields out of struct device into a sub-struct >>> only allocated for DMA capable busses) >> >> I like this idea. In fact, my WIP topic branch now moves the swiotlb >> fields into a separate struct, > > As you have noticed, I have removed that commit again in v2. > > The reason is that I'm not sure about the intended goal. I have looked > around for examples of moving fields out of struct device and found > different approaches: > > A. struct dev_msi_info > The MSI fields are merely grouped in a separate struct, which is > defined in device.h and embedded in struct device. I don't see much > benefit. > > B. struct dev_pm_info > This struct is also embedded in struct device, but it is defined in > , which is mentioned in MAINTAINERS. The benefit is that > further changes are reviewed by this maintainer. The downside is > that device.h includes pm.h. > > C. struct dev_pin_info > This struct is merely declared in device.h and defined > pinctrl/devinfo.h (which is not included). Only a pointer to this > struct is stored in struct device. Of course, the pointer must be > initialized (and released) somehow. > > Here my question: What did you want for DMA fields? > > A. Only grouping those fields in their own struct? > B. Or move the definition to another include file (cf. MAINTAINERS)? > C. Or store a pointer in struct device? dev->dma_parms is already this, and IIRC still has some very old comments somewhere about consolidating the other DMA-related fields in there. > Since you mentioned "allocated", it sounds like you want to achieve C, > but: > > 1. Is it worth the extra dereference for every use? > 2. How should the struct be allocated? Presumably not with kmalloc() in > device_initialize(), because I don't know how to determine if a > device is DMA capable this low in the call stack. So, should it be > allocated together with the containing structure? AFAICS this would > mean changing nearly all device drivers... The bus code knows whether it's a DMA-capable bus or not, and as such should already be providing a .dma_configure method and/or performing some initialisation of DMA fields. Many of the ones that would need to are already providing dma_parms, in fact. Thanks, Robin. > > As you can see, I need some more guidance from you before I can start > working on this. ;-) > > Petr T