Message ID | 98ef4f76d7a5f90b0878e649a70b101402b8889d.1689761699.git.mst@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp2339807vqt; Wed, 19 Jul 2023 03:34:24 -0700 (PDT) X-Google-Smtp-Source: APBJJlFORwyTMuNOAieSGuXk/xrddzfUyMbGWkq/JlKFdDeARRzOjxmWn/0AllimlLD5JkUWW1dM X-Received: by 2002:a05:6512:3b83:b0:4fc:3756:754e with SMTP id g3-20020a0565123b8300b004fc3756754emr1921893lfv.56.1689762864262; Wed, 19 Jul 2023 03:34:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689762864; cv=none; d=google.com; s=arc-20160816; b=hVajMwAadaBp8UqzpjUr4q3GOAPG90Jvhy/i0FhZ54RM009Sd7ZqmkV5F72gID9MUu 5CVXf4Wxq5bj7l8yOPCXxH1AnlPwJomp7E45plvEPB/NGzFYGi76bAXyK3wycp6iIvSH /7W4YXipbFlu1UejeSDX/xhfwZ2DDLjT4hx5HGc47N12GZvn1j8NhJvm/Kis6t0uMw1f R2ITWYlrNMQPyJdZ3lIbzVU/xiBttsG+tRwMKoZSL5nxjYRm3EVhCDednJ2NjwsTjDqM OVXZ8skF17paXteJDzmv4muWz4sYednjPbeBOVBWGsO9f7nNnw/b/LYKrpbPVSGMdvcZ Hqog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-disposition:mime-version:message-id :subject:cc:to:from:date:dkim-signature; bh=BuruPS9lhc1t9yWa6oN21bRI3YjjBxzYMDFcnENMa/E=; fh=jXYes7++OtGuw1x+qaYYTTWBxYX92UWx4qQmnLA0GG0=; b=reMsOd9BkDK18vwyT9Q8ZhWlHi9+MDK2VybPV5nZ4esqYWdUCx3SvkT7w3oDsWi6IQ oaWWkwyQyBWMLS0NR4w2+VZ7Mj5YeMR/UMLR4evmCcgt0EgpvzuudNRq1kP15ooGDoW3 fjZkX8pd3iYkvpBxbQJKkfyCVZXi3TsXSvtiAkQejydTz8I5A60wtAnoiKxJcs0xBWLa PFI3TpK3DHRR4Exq88yA5APhZzOIWNW77GyWzJvfJzdy+yxzFid8tA1W9M3+IwjVVcvf 6uyVHLL0sRdXhFuXgm8mVwpUHr/UfZUCu5z7kLtd6tDnho1+Fmvv8tLk52EEBNOi9vjF +mUg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="CmCh/qQz"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id g16-20020aa7c590000000b0051da8586908si2765652edq.405.2023.07.19.03.34.00; Wed, 19 Jul 2023 03:34:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="CmCh/qQz"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229790AbjGSKRG (ORCPT <rfc822;assdfgzxcv4@gmail.com> + 99 others); Wed, 19 Jul 2023 06:17:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39130 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229788AbjGSKRE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 19 Jul 2023 06:17:04 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3DC91FC4 for <linux-kernel@vger.kernel.org>; Wed, 19 Jul 2023 03:16:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689761771; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=BuruPS9lhc1t9yWa6oN21bRI3YjjBxzYMDFcnENMa/E=; b=CmCh/qQzOagEpVIt5ez+uw8TaGeR0uwIjAaVOETlX1e3EpDn78cxUI4UZjOttKi3Gn5v30 L95gFOIOXUz0GMogmsIfMI4XtY145ZHQmhPcZBik6WFrUOIL60x+QmMP7FcUHz9dLQCARs jDA3SN9iB6wfVPbf1tBwSzPS9pDKu4Y= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-445-HhHAsMx2MX-QHLE_5b58wQ-1; Wed, 19 Jul 2023 06:16:09 -0400 X-MC-Unique: HhHAsMx2MX-QHLE_5b58wQ-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-31400956ce8so3670347f8f.3 for <linux-kernel@vger.kernel.org>; Wed, 19 Jul 2023 03:16:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689761768; x=1692353768; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BuruPS9lhc1t9yWa6oN21bRI3YjjBxzYMDFcnENMa/E=; b=Wzsb2P4FrcEhzKbBpIHBJIaWYwN/hJNd8huVrtMLuNSCm2DZ2R2RHOXay42ao6imBL kKxKaQcXOTN+dgnorGAd7peLm0QCBTIj3j58s1LznXP5iryKRzpNoyLlzEpoZHZKn9dx IPD5oza7Y4yCV0MvxidX7EFsYE3h4mlwKsFGgL1mqQCbXGumMSsk8qCAQiUfTOOdSbCp ssIKLyT38M8vt8bl1t714pIyxoHRmeMk4KKqkFWpBIKS2yteVQTs/DWBdz4xn2B5Vygk HnD0rQa9rp/HpnH8mxQmqGiQR6JdugrDYzBAoTaEdfSJFr+4eT1QT8Q6vw6s1tO1T4lA xM8w== X-Gm-Message-State: ABy/qLYicwo5ppUiBxeiES193JAiY7dAVDp6UiAh8ntiOhgvgp9FIvPe hgHUvi7Z+aJ724vK+ePhT6jFtTqc5Ewz/ZheQBEHj+Fa/UmiqZj8g6LiDLdNDhfLtyYnnH7NBii SD72CrHYUXqir72g3wqcxN6N88WaOEKybs1miXDEFJEneTWvA64AUn5YGk+saFjOJ+kz+WEkXT/ v64g== X-Received: by 2002:a5d:49c7:0:b0:315:94e7:e100 with SMTP id t7-20020a5d49c7000000b0031594e7e100mr1612375wrs.55.1689761768357; Wed, 19 Jul 2023 03:16:08 -0700 (PDT) X-Received: by 2002:a5d:49c7:0:b0:315:94e7:e100 with SMTP id t7-20020a5d49c7000000b0031594e7e100mr1612350wrs.55.1689761767914; Wed, 19 Jul 2023 03:16:07 -0700 (PDT) Received: from redhat.com ([2.52.16.41]) by smtp.gmail.com with ESMTPSA id m18-20020a056000181200b0031434936f0dsm4895611wrh.68.2023.07.19.03.16.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Jul 2023 03:16:06 -0700 (PDT) Date: Wed, 19 Jul 2023 06:15:59 -0400 From: "Michael S. Tsirkin" <mst@redhat.com> To: linux-kernel@vger.kernel.org Cc: Jonathan Corbet <corbet@lwn.net>, Christoph Hellwig <hch@lst.de>, Marek Szyprowski <m.szyprowski@samsung.com>, Robin Murphy <robin.murphy@arm.com>, linux-doc@vger.kernel.org, iommu@lists.linux.dev Subject: [PATCH] dma: DMA_ATTR_SKIP_CPU_SYNC documentation tweaks Message-ID: <98ef4f76d7a5f90b0878e649a70b101402b8889d.1689761699.git.mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Mailer: git-send-email 2.27.0.106.g8ac3dc51b1 X-Mutt-Fcc: =sent X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771844785113053341 X-GMAIL-MSGID: 1771844785113053341 |
Series |
dma: DMA_ATTR_SKIP_CPU_SYNC documentation tweaks
|
|
Commit Message
Michael S. Tsirkin
July 19, 2023, 10:15 a.m. UTC
A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC
might be easily misunderstood.
This attempts to improve documentation in several ways:
when used with dma_map_*, DMA_ATTR_SKIP_CPU_SYNC does not
really assume buffer has been transferred previously -
the buffer would often not even exist in device domain
before it's mapped, instead it normally has to be transferred later.
Add a hint on how buffer can be transferred.
Code comments near DMA_ATTR_SKIP_CPU_SYNC focus on
the use-case of CPU cache synchronization while in
practice this flag isn't limited to that.
Make it more generic.
A couple of things I'm thinking about left for a follow-up patch:
- rename DMA_ATTR_SKIP_CPU_SYNC to DMA_ATTR_SKIP_SYNC
there's nothing I can see making it especially related to the CPU.
- drop mentions of CPU cache from documentation completely
and talk about CPU domain exclusively, or maybe mention
CPU cache as an example: CPU domain (e.g. CPU cache).
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Documentation/core-api/dma-attributes.rst | 5 +++--
include/linux/dma-mapping.h | 5 ++---
2 files changed, 5 insertions(+), 5 deletions(-)
Comments
On Wed, Jul 19, 2023 at 06:15:59AM -0400, Michael S. Tsirkin wrote: > A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC > might be easily misunderstood. .. just curious: what patchset is that? DMA_ATTR_SKIP_CPU_SYNC is often a bad idea and all users probably could use a really good audit.. > #define DMA_ATTR_NO_KERNEL_MAPPING (1UL << 4) > /* > - * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of > - * the CPU cache for the given buffer assuming that it has been already > - * transferred to 'device' domain. > + * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of the > + * CPU and device domains for the given buffer. While we're at it, I think "allows" is the wrong word here, we really must skip the synchronization or else we're in trouble.
On Thu, Jul 20, 2023 at 08:07:42AM +0200, Christoph Hellwig wrote: > On Wed, Jul 19, 2023 at 06:15:59AM -0400, Michael S. Tsirkin wrote: > > A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC > > might be easily misunderstood. > > .. just curious: what patchset is that? DMA_ATTR_SKIP_CPU_SYNC is > often a bad idea and all users probably could use a really good > audit.. Message-Id: <20230710034237.12391-1-xuanzhuo@linux.alibaba.com> Looks like there's really little else can be done: there's a shared page we allow DMA into, so we sync periodically. Then when we unmap we really do not need that data synced again. What exactly is wrong with this? > > #define DMA_ATTR_NO_KERNEL_MAPPING (1UL << 4) > > /* > > - * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of > > - * the CPU cache for the given buffer assuming that it has been already > > - * transferred to 'device' domain. > > + * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of the > > + * CPU and device domains for the given buffer. > > While we're at it, I think "allows" is the wrong word here, we really > must skip the synchronization or else we're in trouble. Hmm could you explain? I thought multiple sync operations are harmless.
On Thu, Jul 20, 2023 at 02:21:05AM -0400, Michael S. Tsirkin wrote: > On Thu, Jul 20, 2023 at 08:07:42AM +0200, Christoph Hellwig wrote: > > On Wed, Jul 19, 2023 at 06:15:59AM -0400, Michael S. Tsirkin wrote: > > > A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC > > > might be easily misunderstood. > > > > .. just curious: what patchset is that? DMA_ATTR_SKIP_CPU_SYNC is > > often a bad idea and all users probably could use a really good > > audit.. > > Message-Id: <20230710034237.12391-1-xuanzhuo@linux.alibaba.com> Do you have an actual link? > > > Looks like there's really little else can be done: there's a > shared page we allow DMA into, so we sync periodically. > Then when we unmap we really do not need that data > synced again. > > What exactly is wrong with this? A "shared" page without ownership can't work with the streaming DMA API (dma_map_*) at all. You need to use dma_alloc_coherent so that it is mapped uncached.
On Thu, Jul 20, 2023 at 08:25:25AM +0200, Christoph Hellwig wrote: > On Thu, Jul 20, 2023 at 02:21:05AM -0400, Michael S. Tsirkin wrote: > > On Thu, Jul 20, 2023 at 08:07:42AM +0200, Christoph Hellwig wrote: > > > On Wed, Jul 19, 2023 at 06:15:59AM -0400, Michael S. Tsirkin wrote: > > > > A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC > > > > might be easily misunderstood. > > > > > > .. just curious: what patchset is that? DMA_ATTR_SKIP_CPU_SYNC is > > > often a bad idea and all users probably could use a really good > > > audit.. > > > > Message-Id: <20230710034237.12391-1-xuanzhuo@linux.alibaba.com> > > Do you have an actual link? sure, they are not hard to generate ;) https://lore.kernel.org/all/20230710034237.12391-11-xuanzhuo%40linux.alibaba.com > > > > > > Looks like there's really little else can be done: there's a > > shared page we allow DMA into, so we sync periodically. > > Then when we unmap we really do not need that data > > synced again. > > > > What exactly is wrong with this? > > A "shared" page without ownership can't work with the streaming > DMA API (dma_map_*) at all. You need to use dma_alloc_coherent > so that it is mapped uncached. Hmm confused. Based on both documentation and code I think this works: dma_map dma_sync dma_sync dma_sync dma_sync dma_unmap(DMA_ATTR_SKIP_CPU_SYNC) right?
On Thu, Jul 20, 2023 at 02:30:08AM -0400, Michael S. Tsirkin wrote: > On Thu, Jul 20, 2023 at 08:25:25AM +0200, Christoph Hellwig wrote: > > On Thu, Jul 20, 2023 at 02:21:05AM -0400, Michael S. Tsirkin wrote: > > > On Thu, Jul 20, 2023 at 08:07:42AM +0200, Christoph Hellwig wrote: > > > > On Wed, Jul 19, 2023 at 06:15:59AM -0400, Michael S. Tsirkin wrote: > > > > > A recent patchset highlighted to me that DMA_ATTR_SKIP_CPU_SYNC > > > > > might be easily misunderstood. > > > > > > > > .. just curious: what patchset is that? DMA_ATTR_SKIP_CPU_SYNC is > > > > often a bad idea and all users probably could use a really good > > > > audit.. > > > > > > Message-Id: <20230710034237.12391-1-xuanzhuo@linux.alibaba.com> > > > > Do you have an actual link? > > sure, they are not hard to generate ;) > > https://lore.kernel.org/all/20230710034237.12391-11-xuanzhuo%40linux.alibaba.com actually there's a new version https://lore.kernel.org/all/20230719040422.126357-11-xuanzhuo%40linux.alibaba.com you can see it does map, sync, unmap unmap immediately after sync seems to be exactly the use case for DMA_ATTR_SKIP_CPU_SYNC. > > > > > > > > > Looks like there's really little else can be done: there's a > > > shared page we allow DMA into, so we sync periodically. > > > Then when we unmap we really do not need that data > > > synced again. > > > > > > What exactly is wrong with this? > > > > A "shared" page without ownership can't work with the streaming > > DMA API (dma_map_*) at all. You need to use dma_alloc_coherent > > so that it is mapped uncached. > > Hmm confused. Based on both documentation and code I think this works: > > dma_map > dma_sync > dma_sync > dma_sync > dma_sync > dma_unmap(DMA_ATTR_SKIP_CPU_SYNC) > > right? > > -- > MST
On Thu, Jul 20, 2023 at 02:30:04AM -0400, Michael S. Tsirkin wrote: > sure, they are not hard to generate ;) > > https://lore.kernel.org/all/20230710034237.12391-11-xuanzhuo%40linux.alibaba.com Thanks, I'll chime in there. > > > Looks like there's really little else can be done: there's a > > > shared page we allow DMA into, so we sync periodically. > > > Then when we unmap we really do not need that data > > > synced again. > > > > > > What exactly is wrong with this? > > > > A "shared" page without ownership can't work with the streaming > > DMA API (dma_map_*) at all. You need to use dma_alloc_coherent > > so that it is mapped uncached. > > Hmm confused. Based on both documentation and code I think this works: > > dma_map > dma_sync > dma_sync > dma_sync > dma_sync > dma_unmap(DMA_ATTR_SKIP_CPU_SYNC) > > right? Depends on your definition of "shared". If there is always a clear owner at a given time you can games with lots of syncs that transfer ownership. If there is no clear ownership, and the "device" just DMAs into the buffer at random times and the host checks bits in there we need to map the buffer uncached. I'll chime in in the thread. > > -- > MST ---end quoted text---
On Thu, Jul 20, 2023 at 08:43:18AM +0200, Christoph Hellwig wrote: > On Thu, Jul 20, 2023 at 02:30:04AM -0400, Michael S. Tsirkin wrote: > > sure, they are not hard to generate ;) > > > > https://lore.kernel.org/all/20230710034237.12391-11-xuanzhuo%40linux.alibaba.com > > Thanks, I'll chime in there. > > > > > Looks like there's really little else can be done: there's a > > > > shared page we allow DMA into, so we sync periodically. > > > > Then when we unmap we really do not need that data > > > > synced again. > > > > > > > > What exactly is wrong with this? > > > > > > A "shared" page without ownership can't work with the streaming > > > DMA API (dma_map_*) at all. You need to use dma_alloc_coherent > > > so that it is mapped uncached. > > > > Hmm confused. Based on both documentation and code I think this works: > > > > dma_map > > dma_sync > > dma_sync > > dma_sync > > dma_sync > > dma_unmap(DMA_ATTR_SKIP_CPU_SYNC) > > > > right? > > Depends on your definition of "shared". If there is always a clear > owner at a given time you can games with lots of syncs that transfer > ownership. If there is no clear ownership, and the "device" just > DMAs into the buffer at random times and the host checks bits in > there we need to map the buffer uncached. > > I'll chime in in the thread. Each chunk of that buffer is DMA'd into separately and then sync'd afterwards. > > > > -- > > MST > ---end quoted text---
diff --git a/Documentation/core-api/dma-attributes.rst b/Documentation/core-api/dma-attributes.rst index 1887d92e8e92..782734666790 100644 --- a/Documentation/core-api/dma-attributes.rst +++ b/Documentation/core-api/dma-attributes.rst @@ -61,8 +61,9 @@ same synchronization operation on the CPU cache. CPU cache synchronization might be a time consuming operation, especially if the buffers are large, so it is highly recommended to avoid it if possible. DMA_ATTR_SKIP_CPU_SYNC allows platform code to skip synchronization of -the CPU cache for the given buffer assuming that it has been already -transferred to 'device' domain. This attribute can be also used for +the CPU cache for the given buffer assuming that it is +transferred to 'device' domain separately, e.g. using +dma_sync_{single,sg}_for_{cpu,device}. This attribute can be also used for dma_unmap_{single,page,sg} functions family to force buffer to stay in device domain after releasing a mapping for it. Use this attribute with care! diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 0ee20b764000..13295ae4385a 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -32,9 +32,8 @@ */ #define DMA_ATTR_NO_KERNEL_MAPPING (1UL << 4) /* - * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of - * the CPU cache for the given buffer assuming that it has been already - * transferred to 'device' domain. + * DMA_ATTR_SKIP_CPU_SYNC: Allows platform code to skip synchronization of the + * CPU and device domains for the given buffer. */ #define DMA_ATTR_SKIP_CPU_SYNC (1UL << 5) /*