linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).