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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC1E8EE14DE for ; Thu, 7 Sep 2023 11:12:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CBCC3440187; Thu, 7 Sep 2023 07:12:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C43778E000F; Thu, 7 Sep 2023 07:12:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABD71440187; Thu, 7 Sep 2023 07:12:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 969C68E000F for ; Thu, 7 Sep 2023 07:12:31 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 57944141119 for ; Thu, 7 Sep 2023 11:12:31 +0000 (UTC) X-FDA: 81209538102.19.0D40BE8 Received: from bee.tesarici.cz (bee.tesarici.cz [77.93.223.253]) by imf26.hostedemail.com (Postfix) with ESMTP id 50070140002 for ; Thu, 7 Sep 2023 11:12:28 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=tesarici.cz header.s=mail header.b=UeWu1Ze8; dmarc=pass (policy=none) header.from=tesarici.cz; spf=pass (imf26.hostedemail.com: domain of petr@tesarici.cz designates 77.93.223.253 as permitted sender) smtp.mailfrom=petr@tesarici.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694085148; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=jyt8gYvPFIZThXvsT4LHesVRY8Ys4B3Qy+pPD6F74DM=; b=eYzl6/HizvPo7DcfapKLjXVEvlhL0j6OBLuveYWlW9hyqY1SaIsvVPrkB0EJfe3z0qPN3Q Um3mOX0w6VPDp3LpxGQGb3BCE/UJDaGF6DcowE3Qx+FMuqaw/DSdmbia9O9xKeV7sjW5tk CVfUDsXnA1f/Q77Kp+jNffNsx6x2Be0= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=tesarici.cz header.s=mail header.b=UeWu1Ze8; dmarc=pass (policy=none) header.from=tesarici.cz; spf=pass (imf26.hostedemail.com: domain of petr@tesarici.cz designates 77.93.223.253 as permitted sender) smtp.mailfrom=petr@tesarici.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694085148; a=rsa-sha256; cv=none; b=yLRCwkiJDNaDTZG/4L3sGXLIJyW2f5rPYzFsSrE3DEwgltbrI5n4UHiC5FjLi/IPVjdLbs fQN2ozhF3s5FNq3z62rsl0KZpc1NqowKGL2nWvicyDGKI5Vabh68onQUiDXmj5ic127FJD 7uKolB5PhcAM6wZbul6RAmI7JPHZ25g= Received: from meshulam.tesarici.cz (dynamic-2a00-1028-83b8-1e7a-4427-cc85-6706-c595.ipv6.o2.cz [IPv6:2a00:1028:83b8:1e7a:4427:cc85:6706:c595]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bee.tesarici.cz (Postfix) with ESMTPSA id EA87A18CBB2; Thu, 7 Sep 2023 13:12:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tesarici.cz; s=mail; t=1694085145; bh=fbplRXoTU4SArEuqSCeWcBdBKDzKAfUk7UIh+zzJ294=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=UeWu1Ze8yhdkAQSYLwMwA5Evs8is8jd9+WRDve9QstEAJCVJk4EyPrZUvt2GlulJO Acv8RNSZT4NRo56VbHVjN1Qces1eN30pA/Z8XXCRCYRiG42VIaxfqRB+JiaUWDiSNI 7vNLYKFtmaNGH7exnrZYJl77SUb4GDfp5VfGRbajxxQjYFEMgAhMtBIGZd04dwv9py LsW5QNMFeGMU27vzeZa8jYcSznJj+rq7+myvFYGpQ5xn1ltALy9BQiZ0pdCtmt5J0/ dC6nWffmW3QcD4g/Ya4kxdbnZOi4vIJaCWhouPtZgmgytg1/Um76VBACnHThhWcUwa sohl8kzvr6NyA== Date: Thu, 7 Sep 2023 13:12:23 +0200 From: Petr =?UTF-8?B?VGVzYcWZw61r?= To: Jonathan Corbet Cc: Petr Tesarik , Stefano Stabellini , Russell King , Thomas Bogendoerfer , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" , "H. Peter Anvin" , Greg Kroah-Hartman , "Rafael J. Wysocki" , Juergen Gross , Oleksandr Tyshchenko , Christoph Hellwig , Marek Szyprowski , Robin Murphy , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Petr Tesarik , Andy Shevchenko , Hans de Goede , James Seo , James Clark , Kees Cook , "moderated list:XEN HYPERVISOR ARM" , "moderated list:ARM PORT" , open list , "open list:MIPS" , "open list:XEN SWIOTLB SUBSYSTEM" , "open list:SLAB ALLOCATOR" , Roberto Sassu Subject: Re: [PATCH v7 9/9] swiotlb: search the software IO TLB only if the device makes use of it Message-ID: <20230907131223.587c1a88@meshulam.tesarici.cz> In-Reply-To: <87il8wr348.fsf@meer.lwn.net> References: <87a5uz3ob8.fsf@meer.lwn.net> <87il8wr348.fsf@meer.lwn.net> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 50070140002 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: zwbmd8em8c499h3h6stna68zhfipqq6y X-HE-Tag: 1694085148-871317 X-HE-Meta: U2FsdGVkX18URwFAI1+nLI/8sO1GzGDlaDNsrGnG9tyHkV7csWmMHb4Y/SYczdpjyYGeoYGFR3WfGkInkOA/rpEZOgsSal98OuxwgIGe/DLhCOtdJXg/mvaTHIYn1gXbi0FK4JNArlv3UATd2xnikMwI8GV+Z7wiKorhstN8NMv45qBEYr8odCu3Yi3vDDyeeJcHPfCo7HTvIPqaXIZUnnU6Mj9cW0aBMaC/BMVvYdL/PDgqz0OogDBQoQ84o5d7uQL93Pzkixxq5TcAkqrOJtAzHPote2GVc/Q8vPO+RJhcyv7kJd891S3uCEAzqfR33yASSTEbuLzj1qfCRahB0PJ0I2T5wc35mZ8+IGlqULitx2ldJm2Xyzt6eqbdzW4Gip0uLyTR6ivi70i/2llI54t4KeRxuFijihSEamho6HkMZCs1J3V+v0oY42NZTkNIukV5aZLGAUVczDPO7E/7wwilxgkchk32zCG9lMndXvzjdurqub85Wo+8xvzOkiJkhMZk8xeuVAxESq7EC6FaPEML8sh5sIRQZBojNpiKWbwQSzsoPW60iiXxITRnTF7mPBeB/j+eFjJ/1SVnoIdUUw/8TIxQiHoDH5JHdwOou6j/25i43LbEmJhaWlqgfz949r9eUTsiqbh2B7ZFkHfUu0SjAiopJJwbbviFM94gn+scmugZ1yHv5omc/vX5AXWRMQVfaJO5tJDTaBiSg8/fCo21o/N7RHdUpl0JPCH4y6BQW3Tl5JzD8nrX+wmoFikXJ7bc7umq1L88goiBBO1JVU9I7JWcRpd3Nh8btL5BceRCjMhL7QxxIkYI4rsPd+qga1SVkBQmv/Ey427OS8nHbCUdV4NB47IdyiFc+GUc0Zs3XOZNKO9gC1NwGU/KT7d0DMtVpRLuvnMsKdf6g3txWBWK4gxhJIxvBf1+HOHUaVTxT9/Cspggc+kGb8xLC/4MiTwxM8/AkGFCnaQZv9c ZHnewRCU PV8U6Mog79nfYIw/EkwKAR7F1dXzfsUOmnvZ916bYzI+2iyjlRd9pcyu3ImsJySKDZ1LhYh8oCRw85hRD0vaA7WEmT68p53lRERmbW+53RKOkJVfdwK+i6zwdWRgnhetLY563aiqn1QsO8sszP7Gf1J77uo5SkGnbP4yYw+9VRE+dfc5PX7mzN8FVGZt7HsDcv/32ORG0t3zTsll/gTIRIqnAAWv+ayrvpFb+DzqeQz9/0SVlpmwK5z2lZmeaZg+BKDWqVhhTjQyT5rPiEIrPj2It48n2v/UxD4JhNowvbTbbe+h8gpyAL/J68dzExU8LEjyDLjXlQvd4UTrcV4WkXw3BiFtyxVQAiNGV+Ic4GB4xzJj7DROjtpGL6KA/y09PxpFuWWLChyOdaskwhZYpxRHt1UhL1v7RxjLL6CbqZoW5YIftgWKVeZ/GqxRT2+gTwY7GgfDhyrqAwuwNXD+FtvWfuS0g0GFOpIRg 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: Hi all, sorry for my late reply; I've been away from my work setup for a month... On Wed, 30 Aug 2023 08:55:51 -0600 Jonathan Corbet wrote: > So it seems this code got merged without this question ever being > answered. Sorry if it's a dumb one, but I don't think this > functionality works as advertised... Yes, I believe the check was originally in is_swiotlb_buffer(), but it got lost during one of the numerous rebases of this patch set. Let me send a follow-up patch after making sure it actually works. Petr T > Thanks, > > jon > > Jonathan Corbet writes: > > > Petr Tesarik writes: > > > >> From: Petr Tesarik > >> > >> Skip searching the software IO TLB if a device has never used it, > >> making sure these devices are not affected by the introduction of > >> multiple IO TLB memory pools. > >> > >> Additional memory barrier is required to ensure that the new value > >> of the flag is visible to other CPUs after mapping a new bounce > >> buffer. For efficiency, the flag check should be inlined, and then > >> the memory barrier must be moved to is_swiotlb_buffer(). However, > >> it can replace the existing barrier in swiotlb_find_pool(), > >> because all callers use is_swiotlb_buffer() first to verify that > >> the buffer address belongs to the software IO TLB. > >> > >> Signed-off-by: Petr Tesarik > >> --- > > > > Excuse me if this is a silly question, but I'm not able to figure > > it out on my own... > > > >> include/linux/device.h | 2 ++ > >> include/linux/swiotlb.h | 7 ++++++- > >> kernel/dma/swiotlb.c | 14 ++++++-------- > >> 3 files changed, 14 insertions(+), 9 deletions(-) > >> > >> diff --git a/include/linux/device.h b/include/linux/device.h > >> index 5fd89c9d005c..6fc808d22bfd 100644 > >> --- a/include/linux/device.h > >> +++ b/include/linux/device.h > >> @@ -628,6 +628,7 @@ struct device_physical_location { > >> * @dma_io_tlb_mem: Software IO TLB allocator. Not for driver > >> use. > >> * @dma_io_tlb_pools: List of transient swiotlb memory > >> pools. > >> * @dma_io_tlb_lock: Protects changes to the list of > >> active pools. > >> + * @dma_uses_io_tlb: %true if device has used the software IO TLB. > >> * @archdata: For arch-specific additions. > >> * @of_node: Associated device tree node. > >> * @fwnode: Associated device node supplied by platform > >> firmware. @@ -737,6 +738,7 @@ struct device { > >> #ifdef CONFIG_SWIOTLB_DYNAMIC > >> struct list_head dma_io_tlb_pools; > >> spinlock_t dma_io_tlb_lock; > >> + bool dma_uses_io_tlb; > > > > You add this new member here, fine... > > > >> #endif > >> /* arch specific additions */ > >> struct dev_archdata archdata; > >> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h > >> index 8371c92a0271..b4536626f8ff 100644 > >> --- a/include/linux/swiotlb.h > >> +++ b/include/linux/swiotlb.h > >> @@ -172,8 +172,13 @@ static inline bool is_swiotlb_buffer(struct > >> device *dev, phys_addr_t paddr) if (!mem) > >> return false; > >> > >> - if (IS_ENABLED(CONFIG_SWIOTLB_DYNAMIC)) > >> + if (IS_ENABLED(CONFIG_SWIOTLB_DYNAMIC)) { > >> + /* Pairs with smp_wmb() in swiotlb_find_slots() > >> and > >> + * swiotlb_dyn_alloc(), which modify the RCU > >> lists. > >> + */ > >> + smp_rmb(); > >> return swiotlb_find_pool(dev, paddr); > >> + } > >> return paddr >= mem->defpool.start && paddr < > >> mem->defpool.end; } > >> > >> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c > >> index adf80dec42d7..d7eac84f975b 100644 > >> --- a/kernel/dma/swiotlb.c > >> +++ b/kernel/dma/swiotlb.c > >> @@ -730,7 +730,7 @@ static void swiotlb_dyn_alloc(struct > >> work_struct *work) > >> add_mem_pool(mem, pool); > >> > >> - /* Pairs with smp_rmb() in swiotlb_find_pool(). */ > >> + /* Pairs with smp_rmb() in is_swiotlb_buffer(). */ > >> smp_wmb(); > >> } > >> > >> @@ -764,11 +764,6 @@ struct io_tlb_pool *swiotlb_find_pool(struct > >> device *dev, phys_addr_t paddr) struct io_tlb_mem *mem = > >> dev->dma_io_tlb_mem; struct io_tlb_pool *pool; > >> > >> - /* Pairs with smp_wmb() in swiotlb_find_slots() and > >> - * swiotlb_dyn_alloc(), which modify the RCU lists. > >> - */ > >> - smp_rmb(); > >> - > >> rcu_read_lock(); > >> list_for_each_entry_rcu(pool, &mem->pools, node) { > >> if (paddr >= pool->start && paddr < pool->end) > >> @@ -813,6 +808,7 @@ void swiotlb_dev_init(struct device *dev) > >> #ifdef CONFIG_SWIOTLB_DYNAMIC > >> INIT_LIST_HEAD(&dev->dma_io_tlb_pools); > >> spin_lock_init(&dev->dma_io_tlb_lock); > >> + dev->dma_uses_io_tlb = false; > > > > ...here you initialize it, fine... > > > >> #endif > >> } > >> > >> @@ -1157,9 +1153,11 @@ static int swiotlb_find_slots(struct device > >> *dev, phys_addr_t orig_addr, list_add_rcu(&pool->node, > >> &dev->dma_io_tlb_pools); > >> spin_unlock_irqrestore(&dev->dma_io_tlb_lock, flags); > >> - /* Pairs with smp_rmb() in swiotlb_find_pool(). */ > >> - smp_wmb(); > >> found: > >> + dev->dma_uses_io_tlb = true; > >> + /* Pairs with smp_rmb() in is_swiotlb_buffer() */ > >> + smp_wmb(); > >> + > > > > ...and here you set it if swiotlb is used. > > > > But, as far as I can tell, you don't actually *use* this field > > anywhere. What am I missing? > > > > Thanks, > > > > jon