All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Howard Yen <howardyen@google.com>, Christoph Hellwig <hch@lst.de>
Cc: m.szyprowski@samsung.com, gregkh@linuxfoundation.org,
	andriy.shevchenko@linux.intel.com, rafael@kernel.org,
	broonie@kernel.org, james@equiv.tech, james.clark@arm.com,
	masahiroy@kernel.org, linux-kernel@vger.kernel.org,
	iommu@lists.linux.dev
Subject: Re: [PATCH v3] dma-coherent: add support for multi coherent rmems per dev
Date: Tue, 27 Feb 2024 14:31:54 +0000	[thread overview]
Message-ID: <602448f1-60a9-41a1-85d2-1205250985d3@arm.com> (raw)
In-Reply-To: <CAJDAHvZ4-mh8uMyq0NiPgWKGt=pS3teoJ0=ofCKZmLeqLXUVgA@mail.gmail.com>

On 27/02/2024 1:39 pm, Howard Yen wrote:
> On Fri, Feb 23, 2024 at 2:37 PM Christoph Hellwig <hch@lst.de> wrote:
>>
>> On Wed, Feb 21, 2024 at 05:27:58PM +0800, Howard Yen wrote:
>>> The reason why I tried to propose this patch is that in the system I'm
>>> working on, where the driver utilizes the coherent reserved memory in
>>> the subsystem for DMA, which has limited memory space as its primary
>>> usage. During the execution of the driver, there is a possibility of
>>> encountering memory depletion scenarios with the primary one.
>>>
>>> To address this issue, I tried to create a patch that enables the
>>> coherent reserved memory driver to support multiple coherent reserved
>>> memory regions per device. This modification aims to provide the
>>> driver with the ability to search for memory from a secondary region
>>> if the primary memory is exhausted, and so on.
>>
>> This all seems pretty vague.  Can you point to your driver submission
>> and explain why it can't just use a larger region instead of multiple
>> smaller ones?
>>
> 
> The reason why it needs multiple regions is that in my system there is
> an always-on subsystem which includes a small size memory, and several
> functions need to run and occupy the memory from the small memory if
> they need to run on the always-on subsystem. These functions must
> allocate the memory from the small memory region, so that they can get
> benefit from the always-on subsystem. So the small memory is split for
> multiple functions which are satisfied with their generic use cases.

I don't really see how that aligns with what you've implemented, though. 
The coherent allocator doesn't have any notion of the caller's use-case, 
it's just going to allocate from wherever it happens to find space 
first. Thus even the calls which would somehow benefit from allocating 
from the "primary" region will have no way to guarantee that they will 
actually allocate from there if it's already been consumed by callers 
who didn't need that benefit but just happened to get there first.

Really that sounds like a case for having specific named memory-regions 
and managing them directly from the relevant driver, rather than trying 
to convince the generic dma-coherent abstraction to do things that don't 
really fit it. Otherwise I'd be slightly concerned that you're expecting 
to bake secret undocumented ABI into DMA API implementations where some 
particular order of allocations must guarantee a particular 
deterministic behaviour, which is really not something we want.

Thanks,
Robin.

> But in specific use cases, they required more memory than their
> pre-allocated memory region, so I tried to propose this patch to give
> it the ability to get the memory from another larger memory to solve
> the issue.
> 
> I'll upload the next version to show the modification in the driver.
> 
> ---
> Best Regards,
> 
> Howard

  reply	other threads:[~2024-02-27 14:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-08 15:28 [PATCH v3] dma-coherent: add support for multi coherent rmems per dev Howard Yen
2024-02-08 18:02 ` Andy Shevchenko
2024-02-19 11:29   ` Howard Yen
2024-02-13  5:54 ` Christoph Hellwig
2024-02-19 11:12   ` Howard Yen
2024-02-20  5:52     ` Christoph Hellwig
2024-02-21  9:27       ` Howard Yen
2024-02-23  6:37         ` Christoph Hellwig
2024-02-27 13:39           ` Howard Yen
2024-02-27 14:31             ` Robin Murphy [this message]
2024-03-04  9:47               ` Howard Yen
2024-02-19 11:27   ` Howard Yen

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=602448f1-60a9-41a1-85d2-1205250985d3@arm.com \
    --to=robin.murphy@arm.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=howardyen@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=james.clark@arm.com \
    --cc=james@equiv.tech \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=masahiroy@kernel.org \
    --cc=rafael@kernel.org \
    /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.