mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Alexey Brodkin <alexey.brodkin@synopsys.com>,
	Will Deacon <will@kernel.org>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Tejun Heo <tj@kernel.org>, Mark Brown <broonie@kernel.org>
Subject: Re: [RFC PATCH v1] devres: align devres.data strictly only for devm_kmalloc()
Date: Fri, 20 Dec 2019 22:02:13 +0000	[thread overview]
Message-ID: <b2e0e322-a4e7-af26-d64a-1ba226e48476@arm.com> (raw)
In-Reply-To: <20191220171359.GP2827@hirez.programming.kicks-ass.net>

On 2019-12-20 5:13 pm, Peter Zijlstra wrote:
> On Fri, Dec 20, 2019 at 03:01:03PM +0000, Robin Murphy wrote:
>> On 2019-12-20 2:06 pm, Peter Zijlstra wrote:
> 
>>> 	data = kmalloc(size + sizeof(struct devres), GFP_KERNEL);

[ afterthought: size could also legitimately be smaller than a pointer 
or some odd length such that the necessary alignment of struct devres 
itself isn't met ]

>> At this point, you'd still need to special-case devm_kmalloc() to ensure
>> size is rounded up to the next ARCH_KMALLOC_MINALIGN granule, or you'd go
>> back to the original problem of the struct devres fields potentially sharing
>> a cache line with the data buffer. That needs to be avoided, because if the
>> devres list is modified while the buffer is mapped for noncoherent DMA
>> (which could legitimately happen as they are nominally distinct allocations
>> with different owners) there's liable to be data corruption one way or the
>> other.
> 
> Wait up, why are you allowing non-coherent DMA at less than page size
> granularity? Is that really sane? Is this really supported behaviour for
> devm ?

There are two DMA APIs - the coherent (or "consistent") API for 
allocating buffers which are guaranteed safe for random access at any 
time *is* page-based, and on non-coherent architectures typically 
involves a non-cacheable remap. There is also the streaming API for 
one-off transfers of data already existing at a given kernel address 
(think network packets, USB URBs, etc), which on non-coherent 
architectures is achieved with explicit cache maintenance plus an API 
contract that buffers must not be explicitly accessed by CPUs for the 
duration of the mapping. Addresses from kmalloc() are explicitly valid 
for dma_map_single() (and indeed are about the only thing you'd ever 
reasonably feed it), which is the primary reason why 
ARCH_KMALLOC_MINALIGN gets so big on architectures which can be 
non-coherent and also suffer from creative cache designs.

See DMA-API.txt and DMA-API-HOWTO.txt in Documentation/ for more details 
if you like.

Robin.

  reply	other threads:[~2019-12-20 22:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-17 15:30 Marc Gonzalez
2019-12-17 15:45 ` Greg Kroah-Hartman
2019-12-17 16:17 ` Marc Gonzalez
2019-12-18 14:20 ` Alexey Brodkin
2019-12-18 15:40   ` Marc Gonzalez
2019-12-20 10:19 ` Marc Gonzalez
2019-12-20 10:22   ` Greg Kroah-Hartman
2019-12-20 10:22     ` Greg Kroah-Hartman
2019-12-20 12:05       ` Marc Gonzalez
2019-12-20 17:19         ` Peter Zijlstra
2019-12-20 14:06   ` Peter Zijlstra
2019-12-20 14:16     ` Greg Kroah-Hartman
2019-12-20 15:01     ` Robin Murphy
2019-12-20 17:13       ` Peter Zijlstra
2019-12-20 22:02         ` Robin Murphy [this message]
2020-01-06 10:05           ` Peter Zijlstra
2019-12-20 19:32       ` Alexey Brodkin
2019-12-20 20:23         ` Peter Zijlstra
2019-12-20 21:02           ` Alexey Brodkin
2019-12-20 21:47             ` Dmitry Torokhov

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=b2e0e322-a4e7-af26-d64a-1ba226e48476@arm.com \
    --to=robin.murphy@arm.com \
    --cc=alexey.brodkin@synopsys.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.w.gonzalez@free.fr \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=tj@kernel.org \
    --cc=will@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome