From: Joerg Roedel <joerg.roedel@amd.com>
To: Shuah Khan <shuah.khan@hp.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Greg KH <greg@kroah.com>, <tglx@linutronix.de>,
<mingo@redhat.com>, <hpa@zytor.com>, <rob@landley.net>,
<akpm@linux-foundation.org>, <bhelgaas@google.com>,
<stern@rowland.harvard.edu>, LKML <linux-kernel@vger.kernel.org>,
<linux-doc@vger.kernel.org>, <devel@linuxdriverproject.org>,
<x86@kernel.org>, <shuahkhan@gmail.com>
Subject: Re: [PATCH] dma-debug: New interfaces to debug dma mapping errors
Date: Wed, 19 Sep 2012 15:08:59 +0200 [thread overview]
Message-ID: <20120919130859.GR2505@amd.com> (raw)
In-Reply-To: <1347997369.2747.68.camel@lorien2>
On Tue, Sep 18, 2012 at 01:42:49PM -0600, Shuah Khan wrote:
> Are you ok with the system wide and per device error counts I added? Any
> comments on the overall approach?
The general approach of having error counters is fine. But the addresses
allocated/addresses checked thing should be done per allocation and not
with counter comparison for several reasons:
1. When doing it per-allocation we know exactly which allocation
was not checked and can tell the driver developer. The code
saves stack-traces for that. This is much more useful than
telling the developer 'somewhere you do not check your
dma-handles'
2. Checking this per-allocation gives you the per-device and
also the per-driver checking you want.
3. You don't need to change 'struct device' for that.
There are more reasons, like that this approach fits a lot better to the
general idea of the DMA-API debugging code.
> The approach you suggested will cover the cases where drivers fail to
> check good map cases. We won't able to catch failed maps that get used
> without checks. Are you not concerned about these cases? These could
> cause a silent error with wild writes or could bring the system down. Or
> are you recommending changing the infrastructure to track failed maps as
> well?
It is fine to only check the good-map cases. Think about what
DMA-debugging is good for: It is a tool for driver developers to find
bugs in their code they wouldn't notice otherwise. An unchecked bad-map
case is a bug they would notice otherwise. So if we check only the
good-map cases and warn the driver developers about non-checked
addresses they fix it and make the drivers more robust against failed
allocations, fixing also the bad-map cases.
> I am still pursuing a way to track failed map cases. I combined the flag
> idea with one of the ideas I am looking into. Details below: (if this
> sounds like a reasonable approach, I can do v2 patch and we can discuss
> the code)
Why do you want to track the bad-map cases?
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
next prev parent reply other threads:[~2012-09-19 13:09 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-17 0:52 [PATCH] dma-debug: New interfaces to debug dma mapping errors Shuah Khan
2012-09-17 2:07 ` Greg KH
2012-09-17 14:45 ` Shuah Khan
2012-09-17 15:25 ` Greg KH
2012-09-17 13:39 ` Konrad Rzeszutek Wilk
2012-09-17 15:52 ` Shuah Khan
2012-09-17 17:23 ` Konrad Rzeszutek Wilk
2012-09-17 22:45 ` Shuah Khan
2012-09-18 13:34 ` Joerg Roedel
2012-09-18 19:42 ` Shuah Khan
2012-09-18 19:45 ` Konrad Rzeszutek Wilk
2012-09-18 20:34 ` Shuah Khan
2012-09-19 13:08 ` Joerg Roedel [this message]
2012-09-19 19:16 ` Shuah Khan
2012-09-26 1:05 ` [PATCH v2] " Shuah Khan
2012-09-26 13:12 ` Konrad Rzeszutek Wilk
2012-09-26 16:23 ` Shuah Khan
2012-09-27 10:20 ` Joerg Roedel
2012-09-27 14:13 ` Shuah Khan
2012-10-03 14:55 ` [PATCH v3] " Shuah Khan
2012-10-03 21:45 ` Andrew Morton
2012-10-04 14:01 ` Konrad Rzeszutek Wilk
2012-10-04 22:16 ` Shuah Khan
2012-10-04 17:38 ` Konrad Rzeszutek Wilk
2012-10-04 22:19 ` Shuah Khan
2012-10-05 1:23 ` [PATCH v4] " Shuah Khan
2012-10-05 22:51 ` Andrew Morton
2012-10-08 17:07 ` Shuah Khan
2012-10-09 21:02 ` Andrew Morton
2012-10-10 21:50 ` Shuah Khan
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=20120919130859.GR2505@amd.com \
--to=joerg.roedel@amd.com \
--cc=akpm@linux-foundation.org \
--cc=bhelgaas@google.com \
--cc=devel@linuxdriverproject.org \
--cc=greg@kroah.com \
--cc=hpa@zytor.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rob@landley.net \
--cc=shuah.khan@hp.com \
--cc=shuahkhan@gmail.com \
--cc=stern@rowland.harvard.edu \
--cc=tglx@linutronix.de \
--cc=x86@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;
as well as URLs for NNTP newsgroup(s).