netdev - Re: [patch 0/5][RFC] Update network drivers to use devres

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070803120745.2d89c221@oldman.hamilton.local>
Date:	Fri, 3 Aug 2007 12:07:45 +0100
From:	Stephen Hemminger <shemminger@...ux-foundation.org>
To:	Tejun Heo <htejun@...il.com>
Cc:	Brandon Philips <brandon@...p.org>, netdev@...r.kernel.org,
	teheo@...e.de
Subject: Re: [patch 0/5][RFC] Update network drivers to use devres

On Fri, 3 Aug 2007 19:26:45 +0900
Tejun Heo <htejun@...il.com> wrote:

> On Fri, Aug 03, 2007 at 09:58:57AM +0100, Stephen Hemminger wrote:
> > On Thu, 2 Aug 2007 15:42:06 -0700
> > Brandon Philips <brandon@...p.org> wrote:
> > 
> > > This patch set adds support for devres in the net core and converts the
> > > e100 and e1000 drivers to devres.  Devres is a simple resource manager
> > > for device drivers, see Documentation/driver-model/devres.txt for more
> > > information.
> > > 
> > > The use of devres will remain optional for drivers with this patch set.
> > > Drivers can be converted when it makes sense.
> > 
> > Just because devres exists is not sufficient motivation to change.
> > 
> > It seems that devres was a band-aid rather than fixing storage drivers
> > to have proper DMA lifetimes.
> 
> I don't really get what you mean by "having proper DMA lifetimes" but
> please don't write devres off too fast.  devres doesn't solve any
> problem that you can't fix without it but it does make the 'solving'
> much easier.
> 
> IMHO, libata drivers generally have been well maintained and reviewed
> but I could still find quite a few bugs (resource leaks or
> occasionally double free) in init failure and removal paths.  Init
> failure paths are especially prone to bugs because they don't get
> excercised often.  It's just very easy to make a mistake and fail to
> notice and low level drivers don't always get sufficient amount of
> review or testing.
> 
> Skimming through drivers... via-rhine doesn't disable PCI device on
> init failure path but does so on removal.  sky2 doesn't free
> consistent memory if sky2_init() fails.  acenic calls iounmap() with
> NULL parameter which I'm not sure whether it's safe or not.  natsemi
> doesn't disable PCI device on failure or removal.

Did you report these to the developers?

> Devres makes low level drivers simpler, easier to get right and
> maintain.  Writing new drivers becomes easier too.  So, why not?
> 
> > Network devices seem to work fine thanks, and the resource requirements
> > are different. If ain't broke, don't fix it.
> 
> Care to enlighten me on how the resource requirments are different
> from ATA drivers?

I was thinking of the hot remove (no mod ref counts) and lingering
/sys open issues.  ATA drivers use ref counts.

My take on devres is that it is similar to talloc() for device drivers.
Not a bad idea in itself, but the real advantage of hierarchical allocation
is that it makes exception handling easier if things are layered deeply.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html