[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1210134804.21644.202.camel@pasglop>
Date: Wed, 07 May 2008 14:33:24 +1000
From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: David Miller <davem@...emloft.net>, tony@...eyournoodle.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Silence 'ignoring return value' warnings in
drivers/video/aty/radeon_base.c
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists