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 1BC28C83F01 for ; Wed, 30 Aug 2023 14:55:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7DB7728004C; Wed, 30 Aug 2023 10:55:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 78BAD280047; Wed, 30 Aug 2023 10:55:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6538A28004C; Wed, 30 Aug 2023 10:55:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 53C5D280047 for ; Wed, 30 Aug 2023 10:55:57 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 2BBD0B22D9 for ; Wed, 30 Aug 2023 14:55:57 +0000 (UTC) X-FDA: 81181070754.22.6995C98 Received: from ms.lwn.net (ms.lwn.net [45.79.88.28]) by imf15.hostedemail.com (Postfix) with ESMTP id C1845A0017 for ; Wed, 30 Aug 2023 14:55:53 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=lwn.net header.s=20201203 header.b=ayp4wECH; dmarc=none; spf=pass (imf15.hostedemail.com: domain of corbet@lwn.net designates 45.79.88.28 as permitted sender) smtp.mailfrom=corbet@lwn.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693407354; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=fzJ2W9IhY60MhTdZXk/DpUQem9Gos7AXFeOTf6bpkgk=; b=2m69YpoLBqFyu1AU2K7ZLVrWrR1PTWGkOj780u3PqJppjQd27K33L7ryCCaRga5uZ4UfOG bHk70pBxyVUtJmiBlvx9UpJKmQ4RyOF0UiY3efbSDIWm356QQ5yQS3tQkGrM4wf7yGu3po 73hPncYtuVdUPPW3DYUE1ZTkMFrWsd0= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=lwn.net header.s=20201203 header.b=ayp4wECH; dmarc=none; spf=pass (imf15.hostedemail.com: domain of corbet@lwn.net designates 45.79.88.28 as permitted sender) smtp.mailfrom=corbet@lwn.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693407354; a=rsa-sha256; cv=none; b=cIMdxSU03FDIiNzph4wH86O+PcSPtbJjXMpYn+zG5YUOREHBaE9emrWSOLrf+d7mJd+7i5 0FU3oSMGgBeWv/cycA6qpcQbwzK/i1ql85Ewe22tljuyEg/nDoi6Lhjz9AIUFIhAAKASFo cfyr/1xU+Kf+1KK+B+pkbtCzU/N57Kc= 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 Content-Type: text/plain X-Rspamd-Queue-Id: C1845A0017 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: s71p93cemthen7er7naabxk4je1fqdak X-HE-Tag: 1693407353-818352 X-HE-Meta: U2FsdGVkX1/TLC63Wmh4tvczDN5CpbZ7gKQMOuGStaBv0t0yqgEykkstvZLSAzU3EoHFuUQ07a+nL5azDqcEjNy4dM4pEVugtEmMY9NkT/723L4MJhW8Fl+K4VoficHO2yPm/JwVDJpaDfDzbkXNybEJyL2vvA9/9RhGGGfQ+9XImQ8gDWjRsmlKyVcY9kv7kzwNrE9kfrLifvw+TTQ0ep5TzrFxvG/HZGfXE4oRxtgh6ixr+bIvEpE5C03RYoP9HJpszhWtsDh5ZM/nC9fUGEWOg+TIE1enorEQmbFysXRa9wD5KE83uof3pqTEeh9mQXVe73Pq1I4009JrzAQ23tAnuTXO05TGOVJWaejX7iZfaGY/kydnhcDdhUsNCAtiYoQ9HdIgLBL7lVP5wUcwp7CFStLMXYSd1ieTf63Ni50jw5GDv+SqWbLYIdsOfrUOYYv+MjchZjm998vtRSkml9sxas41I5LeY4xS9QZLRwQ6fL0ibX9CpXr0FYouOrtb/uHc/IySWT4DsNJD5oy8HRwd7srDrUc6ShJdfN/7PSQN+thulCr6RLqKpPf4hNDWLmSK8ycrrEJyPEaOuZ/6Dwbp6Ux7rpYG8fuQXhXLoOUQU7bzG9AenTNIWf1mnB0nG/JaWgJT6H4kZlSTVvFfIrNIhG0J/575YSjcisam26MD2cJJV1Jp9H9N3dSELievzllfTkm7kFfmQmWmRWLfEg04hxjBUGFiiD2IOefNXMI9zxK2wJJtc2HcTxoy5WcIlKS3wg5qOGzjiP5vAkHnSHssZxtcyQmcZd99/FcFhfN6LpW5K9OZzEkk6vGmSDCYp9emg4wo3mcPI+wPDeU5uCFzGQB9RB0GWh36q7eStmnJAj5gf75RFyPm7zYA2Ux+g4eVRXaEflgxEauwTA6dDvmOYEq8ssnSkDaKMwBvQbThg5NKUof8Ufiw+KXfKPjyx7MAyXIrPkKRUmefrrT j9L7B78P /W7w1WW5c5hlGGDQad3QjFl4lN8zpC8ZXPP+89fd/NML+jPZbcB6RcyeN2j1yK5w+YBgUa+prJ1hO4yuq6a1r4xArhYOmsyqdkrCgsHLzQHOs+ke+XGrbD1+/eyiXSGC42QJb1cyfZGrsACGJx/WdZMJRyhV/YEESO6Y5dYZrXM9J3YIAopBfDQxoat5m7SMeLmC5+SgWm+Ot6s9sTCSXYy7cAInyLDBctwdWDGy6O8/yScz11TCyOuSmqgqrtwTU3lOXIh06Q6eTDRuib/Knpvvd+i5EECmHINiWxxQeY9ksHF6WCUponp+FjTitVuBuAIF8KMMcvWwX3EEn1ti3aLsQbEN5+bc/pILfGJtiimjvaM4z0XxD5UqxBe2K9FKPZtv7fq0cm/8Ang4= 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: 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