All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Barry Song <21cnbao@gmail.com>
Cc: v-songbaohua@oppo.com, zhengtangquan@oppo.com,
	ryan.roberts@arm.com, will@kernel.org, anshuman.khandual@arm.com,
	catalin.marinas@arm.com, linux-kernel@vger.kernel.org,
	surenb@google.com, iommu@lists.linux.dev, maz@kernel.org,
	robin.murphy@arm.com, ardb@kernel.org,
	linux-arm-kernel@lists.infradead.org, m.szyprowski@samsung.com
Subject: Re: [PATCH 5/6] dma-mapping: Allow batched DMA sync operations if supported by the arch
Date: Mon, 22 Dec 2025 10:49:21 +0200	[thread overview]
Message-ID: <20251222084921.GA13529@unreal> (raw)
In-Reply-To: <20251221192458.1320-1-21cnbao@gmail.com>

On Mon, Dec 22, 2025 at 03:24:58AM +0800, Barry Song wrote:
> On Sun, Dec 21, 2025 at 7:55 PM Leon Romanovsky <leon@kernel.org> wrote:
> [...]
> > > +
> >
> > I'm wondering why you don't implement this batch‑sync support inside the
> > arch_sync_dma_*() functions. Doing so would minimize changes to the generic
> > kernel/dma/* code and reduce the amount of #ifdef‑based spaghetti.
> >
> 
> There are two cases: mapping an sg list and mapping a single
> buffer. The former can be batched with
> arch_sync_dma_*_batch_add() and flushed via
> arch_sync_dma_batch_flush(), while the latter requires all work to
> be done inside arch_sync_dma_*(). Therefore,
> arch_sync_dma_*() cannot always batch and flush.

Probably in all cases you can call the _batch_ variant, followed by _flush_,  
even when handling a single page. This keeps the code consistent across all  
paths. On platforms that do not support _batch_, the _flush_ operation will be  
a NOP anyway.

I would also rename arch_sync_dma_batch_flush() to arch_sync_dma_flush().

You can also minimize changes in dma_direct_map_phys() too, by extending
it's signature to provide if flush is needed or not.

dma_direct_map_phys(....) -> dma_direct_map_phys(...., bool flush):

static inline dma_addr_t dma_direct_map_phys(...., bool flush)
{
	....

	if (dma_addr != DMA_MAPPING_ERROR && !dev_is_dma_coherent(dev) &&
	    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
        {
	    	arch_sync_dma_for_device(phys, size, dir);
		if (flush)
			arch_sync_dma_flush();
	}
}

Thanks


WARNING: multiple messages have this Message-ID (diff)
From: Leon Romanovsky <leon@kernel.org>
To: Barry Song <21cnbao@gmail.com>
Cc: ada.coupriediaz@arm.com, anshuman.khandual@arm.com,
	ardb@kernel.org, catalin.marinas@arm.com, iommu@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, m.szyprowski@samsung.com,
	maz@kernel.org, robin.murphy@arm.com, ryan.roberts@arm.com,
	surenb@google.com, v-songbaohua@oppo.com, will@kernel.org,
	zhengtangquan@oppo.com
Subject: Re: [PATCH 5/6] dma-mapping: Allow batched DMA sync operations if supported by the arch
Date: Mon, 22 Dec 2025 10:49:21 +0200	[thread overview]
Message-ID: <20251222084921.GA13529@unreal> (raw)
In-Reply-To: <20251221192458.1320-1-21cnbao@gmail.com>

On Mon, Dec 22, 2025 at 03:24:58AM +0800, Barry Song wrote:
> On Sun, Dec 21, 2025 at 7:55 PM Leon Romanovsky <leon@kernel.org> wrote:
> [...]
> > > +
> >
> > I'm wondering why you don't implement this batch‑sync support inside the
> > arch_sync_dma_*() functions. Doing so would minimize changes to the generic
> > kernel/dma/* code and reduce the amount of #ifdef‑based spaghetti.
> >
> 
> There are two cases: mapping an sg list and mapping a single
> buffer. The former can be batched with
> arch_sync_dma_*_batch_add() and flushed via
> arch_sync_dma_batch_flush(), while the latter requires all work to
> be done inside arch_sync_dma_*(). Therefore,
> arch_sync_dma_*() cannot always batch and flush.

Probably in all cases you can call the _batch_ variant, followed by _flush_,  
even when handling a single page. This keeps the code consistent across all  
paths. On platforms that do not support _batch_, the _flush_ operation will be  
a NOP anyway.

I would also rename arch_sync_dma_batch_flush() to arch_sync_dma_flush().

You can also minimize changes in dma_direct_map_phys() too, by extending
it's signature to provide if flush is needed or not.

dma_direct_map_phys(....) -> dma_direct_map_phys(...., bool flush):

static inline dma_addr_t dma_direct_map_phys(...., bool flush)
{
	....

	if (dma_addr != DMA_MAPPING_ERROR && !dev_is_dma_coherent(dev) &&
	    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
        {
	    	arch_sync_dma_for_device(phys, size, dir);
		if (flush)
			arch_sync_dma_flush();
	}
}

Thanks

  reply	other threads:[~2025-12-22  8:49 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-19  5:36 [PATCH 0/6] dma-mapping: arm64: support batched cache sync Barry Song
2025-12-19  5:36 ` Barry Song
2025-12-19  5:36 ` [PATCH 1/6] arm64: Provide dcache_by_myline_op_nosync helper Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-19 12:20   ` Robin Murphy
2025-12-19 12:20     ` Robin Murphy
2025-12-21  7:22     ` Barry Song
2025-12-21  7:22       ` Barry Song
2025-12-19  5:36 ` [PATCH 2/6] arm64: Provide dcache_clean_poc_nosync helper Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-19  5:36 ` [PATCH 3/6] arm64: Provide dcache_inval_poc_nosync helper Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-19 12:34   ` Robin Murphy
2025-12-19 12:34     ` Robin Murphy
2025-12-21  7:59     ` Barry Song
2025-12-21  7:59       ` Barry Song
2025-12-19  5:36 ` [PATCH 4/6] arm64: Provide arch_sync_dma_ batched helpers Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-19  5:36 ` [PATCH 5/6] dma-mapping: Allow batched DMA sync operations if supported by the arch Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-20 17:37   ` kernel test robot
2025-12-20 17:37     ` kernel test robot
2025-12-21  5:15     ` Barry Song
2025-12-21  5:15       ` Barry Song
2025-12-21 11:55   ` Leon Romanovsky
2025-12-21 11:55     ` Leon Romanovsky
2025-12-21 19:24     ` Barry Song
2025-12-21 19:24       ` Barry Song
2025-12-22  8:49       ` Leon Romanovsky [this message]
2025-12-22  8:49         ` Leon Romanovsky
2025-12-23  0:02         ` Barry Song
2025-12-23  0:02           ` Barry Song
2025-12-23  2:36           ` Barry Song
2025-12-23  2:36             ` Barry Song
2025-12-23 14:14           ` Leon Romanovsky
2025-12-23 14:14             ` Leon Romanovsky
2025-12-24  1:29             ` Barry Song
2025-12-24  1:29               ` Barry Song
2025-12-24  8:51               ` Leon Romanovsky
2025-12-24  8:51                 ` Leon Romanovsky
2025-12-25  5:45                 ` Barry Song
2025-12-25  5:45                   ` Barry Song
2025-12-25 12:36                   ` Leon Romanovsky
2025-12-25 12:36                     ` Leon Romanovsky
2025-12-25 13:31                     ` Barry Song
2025-12-25 13:31                       ` Barry Song
2025-12-25 13:40                       ` Leon Romanovsky
2025-12-25 13:40                         ` Leon Romanovsky
2025-12-21 12:36   ` kernel test robot
2025-12-21 12:36     ` kernel test robot
2025-12-22 12:43   ` kernel test robot
2025-12-22 12:43     ` kernel test robot
2025-12-22 14:00   ` kernel test robot
2025-12-22 14:00     ` kernel test robot
2025-12-19  5:36 ` [PATCH RFC 6/6] dma-iommu: Allow DMA sync batching for IOVA link/unlink Barry Song
2025-12-19  5:36   ` Barry Song
2025-12-19  6:04 ` [PATCH 0/6] dma-mapping: arm64: support batched cache sync Barry Song
2025-12-19  6:04   ` Barry Song
2025-12-19  6:12 ` Barry Song
2025-12-19  6:12   ` Barry Song

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=20251222084921.GA13529@unreal \
    --to=leon@kernel.org \
    --cc=21cnbao@gmail.com \
    --cc=anshuman.khandual@arm.com \
    --cc=ardb@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=iommu@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=maz@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=ryan.roberts@arm.com \
    --cc=surenb@google.com \
    --cc=v-songbaohua@oppo.com \
    --cc=will@kernel.org \
    --cc=zhengtangquan@oppo.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.