From: Robin Murphy <robin.murphy@arm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
"arnd@arndb.de" <arnd@arndb.de>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"sakari.ailus@iki.fi" <sakari.ailus@iki.fi>,
"sumit.semwal@linaro.org" <sumit.semwal@linaro.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"m.szyprowski@samsung.com" <m.szyprowski@samsung.com>
Subject: Re: [PATCH 4/4] dma-debug: Allow poisoning nonzero allocations
Date: Wed, 7 Oct 2015 20:17:03 +0100 [thread overview]
Message-ID: <56156FAF.9020002@arm.com> (raw)
In-Reply-To: <20150929142727.e95a2d2ebff65dda86315248@linux-foundation.org>
On 29/09/15 22:27, Andrew Morton wrote:
[...]
> If I'm understanding things correctly, some allocators zero the memory
> by default and others do not. And we have an unknown number of drivers
> which are assuming that the memory is zeroed.
>
> Correct?
That's precisely the motivation here, yes.
> If so, our options are
>
> a) audit all callers, find the ones which expect zeroed memory but
> aren't passing __GFP_ZERO and fix them.
>
> b) convert all allocators to zero the memory by default.
>
> Obviously, a) is better. How big a job is it?
This I'm not so sure of, hence the very tentative first step. For a very
crude guess at an an upper bound:
$ git grep -E '(dma|pci)_alloc_co(her|nsist)ent' drivers/ | wc -l
1148
vs.
$ git grep -E '(dma|pci)_zalloc_co(her|nsist)ent' drivers/ | wc -l
234
noting that the vast majority of the former are still probably benign,
but picking out those which aren't from the code alone without knowledge
of and/or access to the hardware might be non-trivial.
> This patch will help the process, if people use it.
>
>>>> + if (IS_ENABLED(CONFIG_DMA_API_DEBUG_POISON) && !(flags & __GFP_ZERO))
>>>> + memset(virt, DMA_ALLOC_POISON, size);
>>>> +
>>>
>>> This is likely to be slow in the case of non-cached memory and large
>>> allocations. The config option should come with a warning.
>>
>> It depends on DMA_API_DEBUG, which already has a stern performance
>> warning, is additionally hidden behind EXPERT, and carries a slightly
>> flippant yet largely truthful warning that actually using it could break
>> pretty much every driver in your system; is that not enough?
>
> It might be helpful to provide a runtime knob as well - having to
> rebuild&reinstall just to enable/disable this feature is a bit painful.
Good point - there's always the global DMA debug disable knob, but this
particular feature probably does warrant finer-grained control to be
really practical. Having thought about it some more, it's also probably
wrong that this doesn't respect the dma_debug_driver filter, given that
it is actually invasive; in fixing that, how about if it also *only*
applied when a specific driver is filtered? Then there would be no
problematic "break anything and everything" mode, and the existing
debugfs controls should suffice.
Robin.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
"arnd@arndb.de" <arnd@arndb.de>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"sakari.ailus@iki.fi" <sakari.ailus@iki.fi>,
"sumit.semwal@linaro.org" <sumit.semwal@linaro.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"m.szyprowski@samsung.com" <m.szyprowski@samsung.com>
Subject: Re: [PATCH 4/4] dma-debug: Allow poisoning nonzero allocations
Date: Wed, 7 Oct 2015 20:17:03 +0100 [thread overview]
Message-ID: <56156FAF.9020002@arm.com> (raw)
Message-ID: <20151007191703.n77_IL135sHb6_d5XC_qE7lpjwr-j6BQl6hK0QfhSh0@z> (raw)
In-Reply-To: <20150929142727.e95a2d2ebff65dda86315248@linux-foundation.org>
On 29/09/15 22:27, Andrew Morton wrote:
[...]
> If I'm understanding things correctly, some allocators zero the memory
> by default and others do not. And we have an unknown number of drivers
> which are assuming that the memory is zeroed.
>
> Correct?
That's precisely the motivation here, yes.
> If so, our options are
>
> a) audit all callers, find the ones which expect zeroed memory but
> aren't passing __GFP_ZERO and fix them.
>
> b) convert all allocators to zero the memory by default.
>
> Obviously, a) is better. How big a job is it?
This I'm not so sure of, hence the very tentative first step. For a very
crude guess at an an upper bound:
$ git grep -E '(dma|pci)_alloc_co(her|nsist)ent' drivers/ | wc -l
1148
vs.
$ git grep -E '(dma|pci)_zalloc_co(her|nsist)ent' drivers/ | wc -l
234
noting that the vast majority of the former are still probably benign,
but picking out those which aren't from the code alone without knowledge
of and/or access to the hardware might be non-trivial.
> This patch will help the process, if people use it.
>
>>>> + if (IS_ENABLED(CONFIG_DMA_API_DEBUG_POISON) && !(flags & __GFP_ZERO))
>>>> + memset(virt, DMA_ALLOC_POISON, size);
>>>> +
>>>
>>> This is likely to be slow in the case of non-cached memory and large
>>> allocations. The config option should come with a warning.
>>
>> It depends on DMA_API_DEBUG, which already has a stern performance
>> warning, is additionally hidden behind EXPERT, and carries a slightly
>> flippant yet largely truthful warning that actually using it could break
>> pretty much every driver in your system; is that not enough?
>
> It might be helpful to provide a runtime knob as well - having to
> rebuild&reinstall just to enable/disable this feature is a bit painful.
Good point - there's always the global DMA debug disable knob, but this
particular feature probably does warrant finer-grained control to be
really practical. Having thought about it some more, it's also probably
wrong that this doesn't respect the dma_debug_driver filter, given that
it is actually invasive; in fixing that, how about if it also *only*
applied when a specific driver is filtered? Then there would be no
problematic "break anything and everything" mode, and the existing
debugfs controls should suffice.
Robin.
WARNING: multiple messages have this Message-ID (diff)
From: robin.murphy@arm.com (Robin Murphy)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/4] dma-debug: Allow poisoning nonzero allocations
Date: Wed, 7 Oct 2015 20:17:03 +0100 [thread overview]
Message-ID: <56156FAF.9020002@arm.com> (raw)
In-Reply-To: <20150929142727.e95a2d2ebff65dda86315248@linux-foundation.org>
On 29/09/15 22:27, Andrew Morton wrote:
[...]
> If I'm understanding things correctly, some allocators zero the memory
> by default and others do not. And we have an unknown number of drivers
> which are assuming that the memory is zeroed.
>
> Correct?
That's precisely the motivation here, yes.
> If so, our options are
>
> a) audit all callers, find the ones which expect zeroed memory but
> aren't passing __GFP_ZERO and fix them.
>
> b) convert all allocators to zero the memory by default.
>
> Obviously, a) is better. How big a job is it?
This I'm not so sure of, hence the very tentative first step. For a very
crude guess at an an upper bound:
$ git grep -E '(dma|pci)_alloc_co(her|nsist)ent' drivers/ | wc -l
1148
vs.
$ git grep -E '(dma|pci)_zalloc_co(her|nsist)ent' drivers/ | wc -l
234
noting that the vast majority of the former are still probably benign,
but picking out those which aren't from the code alone without knowledge
of and/or access to the hardware might be non-trivial.
> This patch will help the process, if people use it.
>
>>>> + if (IS_ENABLED(CONFIG_DMA_API_DEBUG_POISON) && !(flags & __GFP_ZERO))
>>>> + memset(virt, DMA_ALLOC_POISON, size);
>>>> +
>>>
>>> This is likely to be slow in the case of non-cached memory and large
>>> allocations. The config option should come with a warning.
>>
>> It depends on DMA_API_DEBUG, which already has a stern performance
>> warning, is additionally hidden behind EXPERT, and carries a slightly
>> flippant yet largely truthful warning that actually using it could break
>> pretty much every driver in your system; is that not enough?
>
> It might be helpful to provide a runtime knob as well - having to
> rebuild&reinstall just to enable/disable this feature is a bit painful.
Good point - there's always the global DMA debug disable knob, but this
particular feature probably does warrant finer-grained control to be
really practical. Having thought about it some more, it's also probably
wrong that this doesn't respect the dma_debug_driver filter, given that
it is actually invasive; in fixing that, how about if it also *only*
applied when a specific driver is filtered? Then there would be no
problematic "break anything and everything" mode, and the existing
debugfs controls should suffice.
Robin.
next prev parent reply other threads:[~2015-10-07 19:17 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-25 12:15 [PATCH 0/4] Assorted DMA mapping tweaks Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` [PATCH 1/4] dmapool: Fix overflow condition in pool_find_page Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` [PATCH 2/4] dma-mapping: Tidy up dma_parms default handling Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` [PATCH 3/4] dma-debug: Check nents in dma_sync_sg* Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` [PATCH 4/4] dma-debug: Allow poisoning nonzero allocations Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:15 ` Robin Murphy
2015-09-25 12:44 ` Russell King - ARM Linux
2015-09-25 12:44 ` Russell King - ARM Linux
2015-09-25 12:44 ` Russell King - ARM Linux
2015-09-25 17:35 ` Robin Murphy
2015-09-25 17:35 ` Robin Murphy
2015-09-25 17:35 ` Robin Murphy
2015-09-29 21:27 ` Andrew Morton
2015-09-29 21:27 ` Andrew Morton
2015-09-29 21:27 ` Andrew Morton
2015-10-07 19:17 ` Robin Murphy [this message]
2015-10-07 19:17 ` Robin Murphy
2015-10-07 19:17 ` Robin Murphy
2015-10-07 19:33 ` Andrew Morton
2015-10-07 19:33 ` Andrew Morton
2015-10-07 19:33 ` Andrew Morton
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=56156FAF.9020002@arm.com \
--to=robin.murphy@arm.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@arm.linux.org.uk \
--cc=m.szyprowski@samsung.com \
--cc=sakari.ailus@iki.fi \
--cc=sumit.semwal@linaro.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.