Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Tomasz Figa <tfiga@chromium.org>
Cc: Christoph Hellwig <hch@lst.de>,
	alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
	linux-mm@kvack.org, Marek Szyprowski <m.szyprowski@samsung.com>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	linux-scsi@vger.kernel.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Ben Skeggs <bskeggs@redhat.com>,
	Matt Porter <mporter@kernel.crashing.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Pawel Osciak <pawel@osciak.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	"list@263.net:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,
	"list@263.net:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	linux-parisc@vger.kernel.org, netdev@vger.kernel.org,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	linux-mips@vger.kernel.org
Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT
Date: Wed, 19 Aug 2020 15:07:04 +0100	[thread overview]
Message-ID: <2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com> (raw)
In-Reply-To: <CAAFQd5AcCTDguB2C9KyDiutXWoEvBL8tL7+a==Uo8vj_8CLOJw@mail.gmail.com>

On 2020-08-19 13:49, Tomasz Figa wrote:
> On Wed, Aug 19, 2020 at 1:51 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> Hi Tomasz,
>>
>> On 2020-08-19 12:16, Tomasz Figa wrote:
>>> Hi Christoph,
>>>
>>> On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig <hch@lst.de> wrote:
>>>>
>>>> The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused,
>>>
>>> Could you explain what makes you think it's unused? It's a feature of
>>> the UAPI generally supported by the videobuf2 framework and relied on
>>> by Chromium OS to get any kind of reasonable performance when
>>> accessing V4L2 buffers in the userspace.
>>>
>>>> and causes
>>>> weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is
>>>> unimplemented except on PARISC and some MIPS configs, and about to be
>>>> removed.
>>>
>>> It is implemented by the generic DMA mapping layer [1], which is used
>>> by a number of architectures including ARM64 and supposed to be used
>>> by new architectures going forward.
>>
>> AFAICS all that V4L2_FLAG_MEMORY_NON_CONSISTENT does is end up
>> controling whether DMA_ATTR_NON_CONSISTENT is added to vb2_queue::dma_attrs.
>>
>> Please can you point to where DMA_ATTR_NON_CONSISTENT does anything at
>> all on arm64?
>>
> 
> With the default config it doesn't, but with
> CONFIG_DMA_NONCOHERENT_CACHE_SYNC enabled it makes dma_pgprot() keep
> the pgprot value as is, without enforcing coherence attributes.

How active are the PA-RISC and MIPS ports of Chromium OS?

Hacking CONFIG_DMA_NONCOHERENT_CACHE_SYNC into an architecture that 
doesn't provide dma_cache_sync() is wrong, since at worst it may break 
other drivers. If downstream is wildly misusing an API then so be it, 
but it's hardly a strong basis for an upstream argument.

>> Also, I posit that videobuf2 is not actually relying on
>> DMA_ATTR_NON_CONSISTENT anyway, since it's clearly not using it properly:
>>
>> "By using this API, you are guaranteeing to the platform
>> that you have all the correct and necessary sync points for this memory
>> in the driver should it choose to return non-consistent memory."
>>
>> $ git grep dma_cache_sync drivers/media
>> $
> 
> AFAIK dma_cache_sync() isn't the only way to perform the cache
> synchronization. The earlier patch series that I reviewed relied on
> dma_get_sgtable() and then dma_sync_sg_*() (which existed in the
> vb2-dc since forever [1]). However, it looks like with the final code
> the sgtable isn't acquired and the synchronization isn't happening, so
> you have a point.

Using the streaming sync calls on coherent allocations has also always 
been wrong per the API, regardless of the bodies of code that have 
happened to get away with it for so long.

> FWIW, I asked back in time what the plan is for non-coherent
> allocations and it seemed like DMA_ATTR_NON_CONSISTENT and
> dma_sync_*() was supposed to be the right thing to go with. [2] The
> same thread also explains why dma_alloc_pages() isn't suitable for the
> users of dma_alloc_attrs() and DMA_ATTR_NON_CONSISTENT.

AFAICS even back then Christoph was implying getting rid of 
NON_CONSISTENT and *replacing* it with something streaming-API-based - 
i.e. this series - not encouraging mixing the existing APIs. It doesn't 
seem impossible to implement a remapping version of this new 
dma_alloc_pages() for IOMMU-backed ops if it's really warranted 
(although at that point it seems like "non-coherent" vb2-dc starts to 
have significant conceptual overlap with vb2-sg).

Robin.

  parent reply	other threads:[~2020-08-19 14:07 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200819065610eucas1p2fde88e81917071b1888e7cc01ba0f298@eucas1p2.samsung.com>
2020-08-19  6:55 ` a saner API for allocating DMA addressable pages Christoph Hellwig
2020-08-19  6:55   ` [PATCH 01/28] mm: turn alloc_pages into an inline function Christoph Hellwig
2020-08-19  6:55   ` [PATCH 02/28] drm/exynos: stop setting DMA_ATTR_NON_CONSISTENT Christoph Hellwig
2020-08-19  6:55   ` [PATCH 03/28] drm/nouveau/gk20a: " Christoph Hellwig
2020-08-19  6:55   ` [PATCH 04/28] net/au1000-eth: stop using DMA_ATTR_NON_CONSISTENT Christoph Hellwig
2020-08-19  6:55   ` [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT Christoph Hellwig
2020-08-19 11:16     ` Tomasz Figa
2020-08-19 11:51       ` Robin Murphy
2020-08-19 12:49         ` Tomasz Figa
2020-08-19 13:57           ` Christoph Hellwig
2020-08-19 14:11             ` Tomasz Figa
2020-08-20  4:45               ` Christoph Hellwig
2020-08-20 10:09                 ` Tomasz Figa
2020-08-20 16:51                   ` Christoph Hellwig
2020-08-19 14:07           ` Robin Murphy [this message]
2020-08-19 14:22             ` Tomasz Figa
2020-08-20  4:52               ` Christoph Hellwig
2020-08-20  5:02             ` Christoph Hellwig
2020-08-20 10:24               ` Tomasz Figa
2020-08-20 16:52                 ` Christoph Hellwig
2020-08-20 17:41                   ` Tomasz Figa
2020-08-19 13:54       ` Christoph Hellwig
2020-08-19 13:57         ` Tomasz Figa
2020-08-20  4:43           ` Christoph Hellwig
2020-08-20  5:20             ` Christoph Hellwig
2020-08-20 10:05               ` Tomasz Figa
2020-08-20 16:54                 ` Christoph Hellwig
2020-08-20 17:33                   ` Tomasz Figa
2020-09-01 11:06                     ` Christoph Hellwig
2020-09-01 15:02                       ` Tomasz Figa
2020-08-19  6:55   ` [PATCH 06/28] lib82596: move DMA allocation into the callers of i82596_probe Christoph Hellwig
2020-09-01 13:29     ` Thomas Bogendoerfer
2020-08-19  6:55   ` [PATCH 07/28] 53c700: improve non-coherent DMA handling Christoph Hellwig
2020-09-01 14:52     ` James Bottomley
2020-09-01 15:05       ` Matthew Wilcox
2020-09-01 15:22         ` James Bottomley
2020-09-01 16:21           ` Helge Deller
2020-09-01 16:41             ` Helge Deller
2020-09-01 16:53               ` Matthew Wilcox
2020-09-02 15:00                 ` Helge Deller
2020-08-19  6:55   ` [PATCH 08/28] MIPS: make dma_sync_*_for_cpu a little less overzealous Christoph Hellwig
2020-09-01 13:53     ` Thomas Bogendoerfer
2020-08-19  6:55   ` [PATCH 09/28] MIPS/jazzdma: remove the unused vdma_remap function Christoph Hellwig
2020-09-01 13:49     ` Thomas Bogendoerfer
2020-08-19  6:55   ` [PATCH 10/28] MIPS/jazzdma: decouple from dma-direct Christoph Hellwig
2020-09-01 13:49     ` Thomas Bogendoerfer
2020-08-19  6:55   ` [PATCH 11/28] dma-mapping: add (back) arch_dma_mark_clean for ia64 Christoph Hellwig
2020-08-19  6:55   ` [PATCH 12/28] dma-direct: remove dma_direct_{alloc,free}_pages Christoph Hellwig
2020-08-19  6:55   ` [PATCH 13/28] dma-direct: lift gfp_t manipulation out of__dma_direct_alloc_pages Christoph Hellwig
2020-08-19  6:55   ` [PATCH 14/28] dma-direct: use phys_to_dma_direct in dma_direct_alloc Christoph Hellwig
2020-08-19  6:55   ` [PATCH 15/28] dma-direct: remove __dma_to_phys Christoph Hellwig
2020-08-19  6:55   ` [PATCH 16/28] dma-direct: rename and cleanup __phys_to_dma Christoph Hellwig
2020-08-19  6:55   ` [PATCH 17/28] dma-mapping: move dma_common_{mmap,get_sgtable} out of mapping.c Christoph Hellwig
2020-08-19  6:55   ` [PATCH 18/28] dma-mapping: move the dma_declare_coherent_memory documentation Christoph Hellwig
2020-08-19  6:55   ` [PATCH 19/28] dma-mapping: replace DMA_ATTR_NON_CONSISTENT with dma_{alloc,free}_pages Christoph Hellwig
2020-08-19 15:03     ` [PATCH 19/28] dma-mapping: replace DMA_ATTR_NON_CONSISTENT with dma_{alloc, free}_pages Tomasz Figa
2020-08-20  5:15       ` Christoph Hellwig
2020-08-19  6:55   ` [PATCH 20/28] sgiwd93: convert from dma_cache_sync to dma_sync_single_for_device Christoph Hellwig
2020-08-19  6:55   ` [PATCH 21/28] hal2: " Christoph Hellwig
2020-08-19  6:55   ` [PATCH 22/28] sgiseeq: " Christoph Hellwig
2020-09-01 15:22     ` Thomas Bogendoerfer
2020-09-01 17:12       ` Thomas Bogendoerfer
2020-09-01 17:16         ` Christoph Hellwig
2020-09-01 17:38           ` Thomas Bogendoerfer
2020-09-02 21:38             ` Thomas Bogendoerfer
2020-09-03  8:42               ` Christoph Hellwig
2020-09-03  8:43             ` Christoph Hellwig
2020-09-03  8:46               ` Christoph Hellwig
2020-08-19  6:55   ` [PATCH 23/28] lib82596: " Christoph Hellwig
2020-08-19  6:55   ` [PATCH 24/28] 53c700: " Christoph Hellwig
2020-08-19  6:55   ` [PATCH 25/28] dma-mapping: remove dma_cache_sync Christoph Hellwig
2020-08-19  6:55   ` [PATCH 26/28] dmapool: add dma_alloc_pages support Christoph Hellwig
2020-08-19  6:55   ` [PATCH 27/28] nvme-pci: fix PRP pool size Christoph Hellwig
2020-08-19  6:55   ` [PATCH 28/28] nvme-pci: use dma_alloc_pages backed dmapools Christoph Hellwig
2020-08-25 11:30   ` a saner API for allocating DMA addressable pages Marek Szyprowski
2020-08-25 13:26     ` Christoph Hellwig
2020-08-29  9:46   ` Helge Deller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2b32f1d8-16f7-3352-40a5-420993d52fb5@arm.com \
    --to=robin.murphy@arm.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=bskeggs@redhat.com \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=jy0922.shim@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mchehab@kernel.org \
    --cc=mporter@kernel.crashing.org \
    --cc=netdev@vger.kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=pawel@osciak.com \
    --cc=sw0312.kim@samsung.com \
    --cc=tfiga@chromium.org \
    --cc=thomas.lendacky@amd.com \
    --cc=tsbogend@alpha.franken.de \
    --subject='Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).