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 266A1C433F5 for ; Thu, 24 Mar 2022 21:14:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354668AbiCXVQO (ORCPT ); Thu, 24 Mar 2022 17:16:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41780 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349749AbiCXVQE (ORCPT ); Thu, 24 Mar 2022 17:16:04 -0400 X-Greylist: delayed 24396 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Thu, 24 Mar 2022 14:14:30 PDT Received: from mail.toke.dk (mail.toke.dk [IPv6:2a0c:4d80:42:2001::664]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 125CF2898C; Thu, 24 Mar 2022 14:14:29 -0700 (PDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1648156468; bh=10YpchtIHtcZ8NSl6ZT7uwLuFlfFgJXa9Oolkv8NtQg=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=bgzPaBmGHN+bnivn53/b7ZIz9/UwYFmY03s5yT9g1LzfZvAJ3uIC7ce11QS/c9gDk R+EyTlbehDyeBvELCV6f+pU+CmP91zr7lx482udUYxg9jrlR/ovTm9K8oIjoB/rEha koK3I9RsZ9nIP/hbajSqIVn+Bk0NpwJEeSXvuQ3T1G3hO5g+mSdwUwK93gKKp92h+F OmmTJoRUd4/LgjWY01urisinyN5elIPMT5iEkPrjHSSPHnq9iwLcPxdf+NOnHNHk0j L5ovl1Ohbmz8Q5MEpZ3NdLlM2L2Qx85IIHFm9o24PgoAXbuYmLr7yHvHkB7jVoVtj+ eFDzUyYMP1/pw== To: Linus Torvalds Cc: Robin Murphy , Christoph Hellwig , Maxime Bizon , Oleksandr Natalenko , Halil Pasic , Marek Szyprowski , Kalle Valo , "David S. Miller" , Jakub Kicinski , Paolo Abeni , Olha Cherevyk , iommu , linux-wireless , Netdev , Linux Kernel Mailing List , Greg Kroah-Hartman , stable Subject: Re: [REGRESSION] Recent swiotlb DMA_FROM_DEVICE fixes break ath9k-based AP In-Reply-To: References: <1812355.tdWV9SEqCh@natalenko.name> <20220324055732.GB12078@lst.de> <4386660.LvFx2qVVIh@natalenko.name> <81ffc753-72aa-6327-b87b-3f11915f2549@arm.com> <878rsza0ih.fsf@toke.dk> <4be26f5d8725cdb016c6fdd9d05cfeb69cdd9e09.camel@freebox.fr> <20220324163132.GB26098@lst.de> <871qyr9t4e.fsf@toke.dk> Date: Thu, 24 Mar 2022 22:14:28 +0100 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87r16r834b.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Linus Torvalds writes: > On Thu, Mar 24, 2022 at 10:07 AM Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> >> Right, but is that sync_for_device call really needed? > > Well, imagine that you have a non-cache-coherent DMA (not bounce > buffers - just bad hardware)... > > So the driver first does that dma_sync_single_for_cpu() for the CPU > see the current state (for the non-cache-coherent case it would just > invalidate caches). > > The driver then examines the command buffer state, sees that it's > still in progress, and does that return -EINPROGRESS. > > It's actually very natural in that situation to flush the caches from > the CPU side again. And so dma_sync_single_for_device() is a fairly > reasonable thing to do in that situation. > > But it doesn't seem *required*, no. The CPU caches only have a copy of > the data in them, no writeback needed (and writeback wouldn't work > since DMA from the device may be in progress). > > So I don't think the dma_sync_single_for_device() is *wrong* per se, > because the CPU didn't actually do any modifications. > > But yes, I think it's unnecessary - because any later CPU accesses > would need that dma_sync_single_for_cpu() anyway, which should > invalidate any stale caches. OK, the above was basically how I understood it. Thank you for confirming! > And it clearly doesn't work in a bounce-buffer situation, but honestly > I don't think a "CPU modified buffers concurrently with DMA" can > *ever* work in that situation, so I think it's wrong for a bounce > buffer model to ever do anything in the dma_sync_single_for_device() > situation. Right. > Does removing that dma_sync_single_for_device() actually fix the > problem for the ath driver? I am hoping Oleksandr can help answer that since my own ath9k hardware is currently on strike :( -Toke