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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id F2146C83F01 for ; Wed, 30 Aug 2023 14:56:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To: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=V3Pfm85NvInIRt1kOCVAjpZSFrMr9LQGmsQ7QD5q3/M=; b=ihfk9/6jFI7Tra WieL1vyE23X0lsbsn9vcjir7ujdZEl5to8gpnzZP0wnKFI/5hGM9Eb6KRUSucPE3udCpqMh+eWdjE mCDK2BgB89e02ofxgT9dwr7J0QHnEGOXcGRSn6sfUctVg++k7EwsWEcxa/HDBVDM87ukbToKc5TRu DR36Gp7V+ADMV59/4EmOF3PQ4cuLlPUSo05/sE8DWUJrrUI2vY2bsUF8UMsb9fZqDnmWuAnRUL/G5 K3+pVpTLU0MyAxJF/NHa7OaGqNq2++K4LJTEYwiUJ0fX2glhJuxZg5vLN4Z0qVWwYi7mKJkW/Z+jt anC4CifCHa4eIoYgclvA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qbMbx-00Di3P-03; Wed, 30 Aug 2023 14:56:01 +0000 Received: from ms.lwn.net ([2600:3c01:e000:3a1::42]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qbMbo-00Di1T-2d for linux-arm-kernel@lists.infradead.org; Wed, 30 Aug 2023 14:55:54 +0000 Received: from localhost (unknown [IPv6:2601:281:8300:73::646]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ms.lwn.net (Postfix) with ESMTPSA id D64B2723; Wed, 30 Aug 2023 14:55:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 ms.lwn.net D64B2723 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lwn.net; s=20201203; t=1693407352; bh=fzJ2W9IhY60MhTdZXk/DpUQem9Gos7AXFeOTf6bpkgk=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=ayp4wECHVmkq6o66QyWfc6SlfuEuvpm+WukmOx5EBL0WA3Tjchi2jo5kGRHspHxVA SwNE0PV6W74pP1HvvyJCHB/HJYXdsaX4+4ZI0su0BVn6R+VI7Tnk+SxakXQgQrDYih Z7CQyMXFkE4YJDU+Om1rR00QEfdnYIWD9fjVr+blRBhgj0LoCL3jNuzNRZhYpqW+p0 cYKX3nqF1lwrwueOku+7j0jjgfhYSo1zZyLEoQmRZNJ3O1yACrBNRDLLvh6G3bfTMY WpF89u4BJ/ycMvRDSiVGQ/AAIYX143xN8fzRqY2GeLgAn1LjYuzkrTUAEkN4Pm5lw+ ttcU/ACVSPYKw== From: Jonathan Corbet To: 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" Cc: Roberto Sassu , petr@tesarici.cz Subject: Re: [PATCH v7 9/9] swiotlb: search the software IO TLB only if the device makes use of it In-Reply-To: <87a5uz3ob8.fsf@meer.lwn.net> References: <87a5uz3ob8.fsf@meer.lwn.net> Date: Wed, 30 Aug 2023 08:55:51 -0600 Message-ID: <87il8wr348.fsf@meer.lwn.net> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230830_075552_858609_CF1E5FD3 X-CRM114-Status: GOOD ( 31.72 ) 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 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... 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel