All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>,
	tony@bakeyournoodle.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Silence 'ignoring return value' warnings in drivers/video/aty/radeon_base.c
Date: Wed, 07 May 2008 14:33:24 +1000	[thread overview]
Message-ID: <1210134804.21644.202.camel@pasglop> (raw)
In-Reply-To: <20080506182006.4b4a3968.akpm@linux-foundation.org>


On Tue, 2008-05-06 at 18:20 -0700, Andrew Morton wrote:
> Look at the history of this.  A couple of years ago we were seeing a huge
> number of mysterious crashes and warnings in and around sysfs and driver
> code and we just didn't have a clue what was causing it, because those
> crashes were happening long, long after the buggy code had done its work.
> 
> So I went in there and found a tremendous amount of code in driver core,
> sysfs and in callers of both which was just ignoring error returns and
> blundering on.
> 
> It was a comnplete undebuggable unmaintainable mess.  And the reason why it
> was undebuggable was because the code was failing to detect errors *when
> they occurred*.  So we (me, Cornelia, Greg, others) set about fixing all of
> that.  And to support that effort we marked all the things which should be
> checked with __must_check.

You haven't read me properly. I'm not advocating completely ignoring
those errors. In fact, I'm all about keeping must check on things like
allocations. However, in cases like sysfs_create_file() like many
similar things where failure will -not- prevent proper operations of the
driver or subsystem, mostly only compromise the user ABI, I believe it's
a _LOT_ more efficient to put -one- printk in the function itself,
rather than all callers

> Now you come along and cherrypick a few callsites where you'd rather not
> bother checking and assert that the entire effort was wrong-headed.  Well
> sorry, no, it wasn't.  Sure, there's a little bit of undesirable fallout
> but the whole thing had.  to.  be.  done.

Of course the whole effort was not wrong headed. I'm really only
complaining about all those stupid sysfs_create_file() and maybe a
handful of similar ones.

Ben.



  reply	other threads:[~2008-05-07  4:34 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-24  4:34 [PATCH] Silence 'ignoring return value' warnings in drivers/video/aty/radeon_base.c Tony Breeds
2008-05-06 21:39 ` Andrew Morton
2008-05-06 21:43   ` David Miller
2008-05-06 21:56     ` Andrew Morton
2008-05-07  4:00       ` Arjan van de Ven
2008-05-07  4:12         ` Andrew Morton
2008-05-07  4:15           ` Arjan van de Ven
2008-05-07  4:20             ` Arjan van de Ven
2008-05-07  4:37               ` Benjamin Herrenschmidt
2008-05-07  4:23             ` Andrew Morton
2008-05-07  4:26               ` Andrew Morton
2008-05-07  4:41                 ` Arjan van de Ven
2008-05-07  4:37               ` Arjan van de Ven
2008-05-07  5:40           ` Paul Mackerras
2008-05-07  5:49             ` Benjamin Herrenschmidt
2008-05-07  0:54     ` Benjamin Herrenschmidt
2008-05-07  1:20       ` Andrew Morton
2008-05-07  4:33         ` Benjamin Herrenschmidt [this message]
2008-05-07  8:23           ` Cornelia Huck
2008-05-07 21:43             ` Benjamin Herrenschmidt
2008-05-08  7:34               ` Cornelia Huck
2008-05-08  7:49                 ` Benjamin Herrenschmidt
2008-05-08  8:36                   ` Cornelia Huck
2008-05-08 22:03                     ` Greg KH
2008-05-08 23:06                       ` Benjamin Herrenschmidt
2008-05-08 23:50                       ` Paul Mackerras
2008-05-09  0:02                         ` Harvey Harrison
2008-05-09  5:32                           ` Cornelia Huck
2008-05-09  5:33                           ` Cornelia Huck

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=1210134804.21644.202.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony@bakeyournoodle.com \
    /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.