All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Petr Tesařík" <petr@tesarici.cz>
To: Christoph Hellwig <hch@lst.de>
Cc: Petr Tesarik <petrtesarik@huaweicloud.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Robin Murphy <robin.murphy@arm.com>, Borislav Petkov <bp@suse.de>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	Kim Phillips <kim.phillips@amd.com>,
	"Steven Rostedt (Google)" <rostedt@goodmis.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:DMA MAPPING HELPERS" <iommu@lists.linux.dev>,
	Roberto Sassu <roberto.sassu@huawei.com>,
	Alexander Graf <graf@amazon.com>
Subject: Re: [RFC v1 3/4] swiotlb: Allow dynamic allocation of bounce buffers
Date: Fri, 21 Apr 2023 15:03:49 +0200	[thread overview]
Message-ID: <20230421150349.35966e0b@meshulam.tesarici.cz> (raw)
In-Reply-To: <20230407121555.4290a011@meshulam.tesarici.cz>

Hi Christoph!

I'd like to follow up on this sub-thread:

On Fri, 7 Apr 2023 12:15:55 +0200
Petr Tesařík <petr@tesarici.cz> wroe:

> On Fri, 7 Apr 2023 07:57:04 +0200
> Christoph Hellwig <hch@lst.de> wrote:
>[...]
> > (Btw, in case anyone is interested, we really need to get started
> > on moving the dma fields out of struct device into a sub-struct
> > only allocated for DMA capable busses)  
> 
> I like this idea. In fact, my WIP topic branch now moves the swiotlb
> fields into a separate struct,

As you have noticed, I have removed that commit again in v2.

The reason is that I'm not sure about the intended goal. I have looked
around for examples of moving fields out of struct device and found
different approaches:

A. struct dev_msi_info
   The MSI fields are merely grouped in a separate struct, which is
   defined in device.h and embedded in struct device. I don't see much
   benefit.

B. struct dev_pm_info
   This struct is also embedded in struct device, but it is defined in
   <linux/pm.h>, which is mentioned in MAINTAINERS. The benefit is that
   further changes are reviewed by this maintainer. The downside is
   that device.h includes pm.h.

C. struct dev_pin_info
   This struct is merely declared in device.h and defined
   pinctrl/devinfo.h (which is not included). Only a pointer to this
   struct is stored in struct device. Of course, the pointer must be
   initialized (and released) somehow.

Here my question: What did you want for DMA fields?

A. Only grouping those fields in their own struct?
B. Or move the definition to another include file (cf. MAINTAINERS)?
C. Or store a pointer in struct device?

Since you mentioned "allocated", it sounds like you want to achieve C,
but:

1. Is it worth the extra dereference for every use?
2. How should the struct be allocated? Presumably not with kmalloc() in
   device_initialize(), because I don't know how to determine if a
   device is DMA capable this low in the call stack. So, should it be
   allocated together with the containing structure? AFAICS this would
   mean changing nearly all device drivers...

As you can see, I need some more guidance from you before I can start
working on this. ;-)

Petr T

  parent reply	other threads:[~2023-04-21 13:04 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20 12:28 [RFC v1 0/4] Allow dynamic allocation of software IO TLB bounce buffers Petr Tesarik
2023-03-20 12:28 ` [RFC v1 1/4] dma-mapping: introduce the DMA_ATTR_MAY_SLEEP attribute Petr Tesarik
2023-03-28  3:57   ` Christoph Hellwig
2023-03-28  7:21     ` Petr Tesarik
2023-04-07  5:52       ` Christoph Hellwig
2023-03-31 13:06   ` Bagas Sanjaya
2023-03-20 12:28 ` [RFC v1 2/4] swiotlb: Move code around in preparation for dynamic bounce buffers Petr Tesarik
2023-03-20 12:28 ` [RFC v1 3/4] swiotlb: Allow dynamic allocation of " Petr Tesarik
2023-03-28  4:07   ` Christoph Hellwig
2023-03-28  7:54     ` Petr Tesarik
2023-03-28 12:43       ` Petr Tesarik
2023-04-07  5:57         ` Christoph Hellwig
2023-04-07 10:15           ` Petr Tesařík
2023-04-13 11:09             ` Petr Tesarik
2023-04-21 13:03             ` Petr Tesařík [this message]
2023-04-21 14:58               ` Robin Murphy
2023-04-21 15:09                 ` Petr Tesařík
2023-04-24  6:03                   ` Christoph Hellwig
2023-03-31  7:26       ` Juerg Haefliger
2023-03-31  9:00         ` Petr Tesařík
2023-04-06 11:44           ` Juerg Haefliger
2023-05-11 10:36             ` Petr Tesařík
2023-04-07  5:55       ` Christoph Hellwig
2023-04-07 10:46         ` Petr Tesařík
2023-04-11  3:51           ` Christoph Hellwig
2023-03-20 12:28 ` [RFC v1 4/4] swiotlb: Add an option to allow dynamic " Petr Tesarik
2023-03-27 11:06 ` [RFC v1 0/4] Allow dynamic allocation of software IO TLB " Petr Tesarik
2023-04-07  6:00   ` Christoph Hellwig

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=20230421150349.35966e0b@meshulam.tesarici.cz \
    --to=petr@tesarici.cz \
    --cc=akpm@linux-foundation.org \
    --cc=bp@suse.de \
    --cc=corbet@lwn.net \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=graf@amazon.com \
    --cc=hch@lst.de \
    --cc=iommu@lists.linux.dev \
    --cc=kim.phillips@amd.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=paulmck@kernel.org \
    --cc=petrtesarik@huaweicloud.com \
    --cc=rdunlap@infradead.org \
    --cc=roberto.sassu@huawei.com \
    --cc=robin.murphy@arm.com \
    --cc=rostedt@goodmis.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.