* [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
@ 2013-11-13 20:56 Shuah Khan
2013-12-30 14:15 ` Joerg Roedel
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2013-11-13 20:56 UTC (permalink / raw)
To: akpm, alexander.h.duyck, joro; +Cc: Shuah Khan, linux-kernel, shuahkhan
dma-debug checks to verify if driver validated the address returned by
dma mapping routines when driver does unmap. If a driver doesn't call
unmap, failure to check mapping errors isn't detected and reported.
Enhancing existing bus notifier_call dma_debug_device_change() to check
for mapping errors at the same time it detects leaked dma buffers for
BUS_NOTIFY_UNBOUND_DRIVER event. It scans for mapping errors and if any
found, prints one warning message that includes mapping error count.
Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
---
lib/dma-debug.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index d87a17a..e34865e 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -717,6 +717,7 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
struct dma_debug_entry *entry;
unsigned long flags;
int count = 0, i;
+ int map_err_cnt = 0;
local_irq_save(flags);
@@ -724,6 +725,8 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
spin_lock(&dma_entry_hash[i].lock);
list_for_each_entry(entry, &dma_entry_hash[i].list, list) {
if (entry->dev == dev) {
+ if (entry->map_err_type == MAP_ERR_NOT_CHECKED)
+ map_err_cnt += 1;
count += 1;
*out_entry = entry;
}
@@ -733,6 +736,10 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
local_irq_restore(flags);
+ if (map_err_cnt)
+ dev_warn(entry->dev,
+ "DMA-API: device driver failed to check map errors: "
+ "[count] = %d\n", map_err_cnt);
return count;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2013-11-13 20:56 [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors Shuah Khan
@ 2013-12-30 14:15 ` Joerg Roedel
2014-01-03 18:26 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Joerg Roedel @ 2013-12-30 14:15 UTC (permalink / raw)
To: Shuah Khan; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan
On Wed, Nov 13, 2013 at 01:56:08PM -0700, Shuah Khan wrote:
> dma-debug checks to verify if driver validated the address returned by
> dma mapping routines when driver does unmap. If a driver doesn't call
> unmap, failure to check mapping errors isn't detected and reported.
>
> Enhancing existing bus notifier_call dma_debug_device_change() to check
> for mapping errors at the same time it detects leaked dma buffers for
> BUS_NOTIFY_UNBOUND_DRIVER event. It scans for mapping errors and if any
> found, prints one warning message that includes mapping error count.
>
> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
> ---
> lib/dma-debug.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> index d87a17a..e34865e 100644
> --- a/lib/dma-debug.c
> +++ b/lib/dma-debug.c
> @@ -717,6 +717,7 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
> struct dma_debug_entry *entry;
> unsigned long flags;
> int count = 0, i;
> + int map_err_cnt = 0;
>
> local_irq_save(flags);
>
> @@ -724,6 +725,8 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
> spin_lock(&dma_entry_hash[i].lock);
> list_for_each_entry(entry, &dma_entry_hash[i].list, list) {
> if (entry->dev == dev) {
> + if (entry->map_err_type == MAP_ERR_NOT_CHECKED)
> + map_err_cnt += 1;
> count += 1;
> *out_entry = entry;
I think it is better to check for this in a seperate function and use
err_printk instead of dev_warn in the end to print the errors.
The new function can then be called in the dma_debug_device_change
callback like device_dma_allocations is.
Joerg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2013-12-30 14:15 ` Joerg Roedel
@ 2014-01-03 18:26 ` Shuah Khan
2014-01-07 14:26 ` Joerg Roedel
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2014-01-03 18:26 UTC (permalink / raw)
To: Joerg Roedel; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan, Shuah Khan
On 12/30/2013 07:15 AM, Joerg Roedel wrote:
> On Wed, Nov 13, 2013 at 01:56:08PM -0700, Shuah Khan wrote:
>> dma-debug checks to verify if driver validated the address returned by
>> dma mapping routines when driver does unmap. If a driver doesn't call
>> unmap, failure to check mapping errors isn't detected and reported.
>>
>> Enhancing existing bus notifier_call dma_debug_device_change() to check
>> for mapping errors at the same time it detects leaked dma buffers for
>> BUS_NOTIFY_UNBOUND_DRIVER event. It scans for mapping errors and if any
>> found, prints one warning message that includes mapping error count.
>>
>> Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
>> ---
>> lib/dma-debug.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/lib/dma-debug.c b/lib/dma-debug.c
>> index d87a17a..e34865e 100644
>> --- a/lib/dma-debug.c
>> +++ b/lib/dma-debug.c
>> @@ -717,6 +717,7 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
>> struct dma_debug_entry *entry;
>> unsigned long flags;
>> int count = 0, i;
>> + int map_err_cnt = 0;
>>
>> local_irq_save(flags);
>>
>> @@ -724,6 +725,8 @@ static int device_dma_allocations(struct device *dev, struct dma_debug_entry **o
>> spin_lock(&dma_entry_hash[i].lock);
>> list_for_each_entry(entry, &dma_entry_hash[i].list, list) {
>> if (entry->dev == dev) {
>> + if (entry->map_err_type == MAP_ERR_NOT_CHECKED)
>> + map_err_cnt += 1;
>> count += 1;
>> *out_entry = entry;
>
> I think it is better to check for this in a seperate function and use
> err_printk instead of dev_warn in the end to print the errors.
> The new function can then be called in the dma_debug_device_change
> callback like device_dma_allocations is.
>
I did explore separate function option and backed off from it since the
new routine will have to duplicate what device_dma_allocations() does
except that it checks for entry->map_err_type.
I still have the patch that does that saved away. If you still prefer
that approach, I can rework the patch and send it.
-- Shuah
--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2014-01-03 18:26 ` Shuah Khan
@ 2014-01-07 14:26 ` Joerg Roedel
2014-01-07 15:00 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Joerg Roedel @ 2014-01-07 14:26 UTC (permalink / raw)
To: Shuah Khan; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan
On Fri, Jan 03, 2014 at 11:26:04AM -0700, Shuah Khan wrote:
> On 12/30/2013 07:15 AM, Joerg Roedel wrote:
> >I think it is better to check for this in a seperate function and use
> >err_printk instead of dev_warn in the end to print the errors.
> >The new function can then be called in the dma_debug_device_change
> >callback like device_dma_allocations is.
> >
>
> I did explore separate function option and backed off from it since
> the new routine will have to duplicate what device_dma_allocations()
> does except that it checks for entry->map_err_type.
>
> I still have the patch that does that saved away. If you still
> prefer that approach, I can rework the patch and send it.
You can get rid of the code duplication by defining for_each macros for
the list traversals. So I would still prefer to have this check in a
seperate function.
Joerg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2014-01-07 14:26 ` Joerg Roedel
@ 2014-01-07 15:00 ` Shuah Khan
2014-01-07 15:12 ` Joerg Roedel
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2014-01-07 15:00 UTC (permalink / raw)
To: Joerg Roedel; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan, Shuah Khan
On 01/07/2014 07:26 AM, Joerg Roedel wrote:
> On Fri, Jan 03, 2014 at 11:26:04AM -0700, Shuah Khan wrote:
>> On 12/30/2013 07:15 AM, Joerg Roedel wrote:
>>> I think it is better to check for this in a seperate function and use
>>> err_printk instead of dev_warn in the end to print the errors.
>>> The new function can then be called in the dma_debug_device_change
>>> callback like device_dma_allocations is.
>>>
>>
>> I did explore separate function option and backed off from it since
>> the new routine will have to duplicate what device_dma_allocations()
>> does except that it checks for entry->map_err_type.
>>
>> I still have the patch that does that saved away. If you still
>> prefer that approach, I can rework the patch and send it.
>
> You can get rid of the code duplication by defining for_each macros for
> the list traversals. So I would still prefer to have this check in a
> seperate function.
>
>
> Joerg
>
>
Joerg,
This patch and a follow-on cocinelli warning fix patch are in
linux-next. Would you like me to send a patch relative to the change in
linux-next or cut a new patch against the latest Linus's git. I can go
either way. We just have to remember to drop those two patches from
linux-next.
-- Shuah
--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2014-01-07 15:00 ` Shuah Khan
@ 2014-01-07 15:12 ` Joerg Roedel
2014-01-07 17:22 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Joerg Roedel @ 2014-01-07 15:12 UTC (permalink / raw)
To: Shuah Khan; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan
On Tue, Jan 07, 2014 at 08:00:33AM -0700, Shuah Khan wrote:
> This patch and a follow-on cocinelli warning fix patch are in
> linux-next. Would you like me to send a patch relative to the change
> in linux-next or cut a new patch against the latest Linus's git. I
> can go either way. We just have to remember to drop those two
> patches from linux-next.
Please do a patch against linus.git and drop the two patches from
linux-next. Over which tree did they go into linux-next anyway?
Joerg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2014-01-07 15:12 ` Joerg Roedel
@ 2014-01-07 17:22 ` Shuah Khan
2014-01-11 0:25 ` Shuah Khan
0 siblings, 1 reply; 8+ messages in thread
From: Shuah Khan @ 2014-01-07 17:22 UTC (permalink / raw)
To: Joerg Roedel; +Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan, Shuah Khan
On 01/07/2014 08:12 AM, Joerg Roedel wrote:
> On Tue, Jan 07, 2014 at 08:00:33AM -0700, Shuah Khan wrote:
>> This patch and a follow-on cocinelli warning fix patch are in
>> linux-next. Would you like me to send a patch relative to the change
>> in linux-next or cut a new patch against the latest Linus's git. I
>> can go either way. We just have to remember to drop those two
>> patches from linux-next.
>
> Please do a patch against linus.git and drop the two patches from
> linux-next. Over which tree did they go into linux-next anyway?
>
>
ok. It went through mm tree.
-- Shuah
--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors
2014-01-07 17:22 ` Shuah Khan
@ 2014-01-11 0:25 ` Shuah Khan
0 siblings, 0 replies; 8+ messages in thread
From: Shuah Khan @ 2014-01-11 0:25 UTC (permalink / raw)
To: Joerg Roedel
Cc: akpm, alexander.h.duyck, linux-kernel, shuahkhan, Shuah Khan, shuahkhan
On 01/07/2014 10:22 AM, Shuah Khan wrote:
> On 01/07/2014 08:12 AM, Joerg Roedel wrote:
>> On Tue, Jan 07, 2014 at 08:00:33AM -0700, Shuah Khan wrote:
>>> This patch and a follow-on cocinelli warning fix patch are in
>>> linux-next. Would you like me to send a patch relative to the change
>>> in linux-next or cut a new patch against the latest Linus's git. I
>>> can go either way. We just have to remember to drop those two
>>> patches from linux-next.
>>
>> Please do a patch against linus.git and drop the two patches from
>> linux-next. Over which tree did they go into linux-next anyway?
>>
>>
>
> ok. It went through mm tree.
>
I have a separate routine now for scanning for entries with map error
flag set and I also have a new err_printk() call to print the entry from
dma_debug_device_change() right after device_dma_allocations() check and
corresponding err_printk(). Snippet of the diff below. Only one warning
gets printed. Looking at err_printk(): if (!show_all_errors &&
show_num_errors > 0) conditional, it appears that is the way it is
supposed to work, unless I am missing something
One way to not miss the second warning is to just generate just a
pr_err() for the mapping error instead of err_printk(). Leaked entries
is a bigger problem that should really be shown and the same entry might
not have the map error flag set. Thoughts?
static int dma_debug_device_change(struct notifier_block *nb, unsigned
long action, void *data)
{
struct device *dev = data;
@@ -758,6 +784,18 @@ static int dma_debug_device_change(struct
notifier_block *nb, unsigned long acti
"[mapped with %s] [mapped as %s]\n",
count, entry->dev_addr, entry->size,
dir2name[entry->direction],
type2name[entry->type]);
+
+ count = device_dma_map_errors(dev, &entry);
+ if (count == 0)
+ break;
+ err_printk(dev, entry,
+ "DMA-API: device driver failed to check
map err"
+ ": [count=%d]\n"
+ "Details of an entry with map error flag
set: "
+ "[device address=0x%016llx] [size=%llu
bytes] "
+ "[mapped with %s] [mapped as %s]\n",
+ count, entry->dev_addr, entry->size,
+ dir2name[entry->direction], type2name[entry->type]);
break;
--
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-01-11 0:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-13 20:56 [PATCH v2] dma-debug: enhance dma_debug_device_change() to check for mapping errors Shuah Khan
2013-12-30 14:15 ` Joerg Roedel
2014-01-03 18:26 ` Shuah Khan
2014-01-07 14:26 ` Joerg Roedel
2014-01-07 15:00 ` Shuah Khan
2014-01-07 15:12 ` Joerg Roedel
2014-01-07 17:22 ` Shuah Khan
2014-01-11 0:25 ` Shuah Khan
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.