From: Tejun Heo <htejun@gmail.com>
To: Stephen Hemminger <shemminger@linux-foundation.org>
Cc: Brandon Philips <brandon@ifup.org>,
netdev@vger.kernel.org, teheo@suse.de
Subject: Re: [patch 0/5][RFC] Update network drivers to use devres
Date: Fri, 03 Aug 2007 20:33:04 +0900 [thread overview]
Message-ID: <46B31270.7090503@gmail.com> (raw)
In-Reply-To: <20070803120745.2d89c221@oldman.hamilton.local>
Hello,
Stephen Hemminger wrote:
>> 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?
Just skimmed through. I'm pretty sure Brandon will pick those up later.
>> 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.
I guess the hot removing is done by severing netdev from the actual
device, right? I don't see how that affects usage of devres on network
drivers. Am I missing something?
On a separate note, can you explain lingering /sys open issue to me a
bit? With recent sysfs changes, sysfs nodes are disconnected
immediately on deletion. Would that make any difference to netdevs?
> 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.
Yeah, devres made layering easier in libata, especially SFF stuff.
Dunno how much of that is applicable to netdev but, with or without
layering, it'll be a nice cleanup and I don't see much negative side.
Conversion would take some work and bugs might be introduced in the
process as with any changes but the good thing about devres is that
you're very likely to get failure/release paths right if you get the
init path right, and if you get the init path wrong, it will stand out
like a sore thumb - easy to spot, easy to fix.
So, I think using devres on net drivers is a good idea, well, for that
matter, for any driver, but me being the devres writer, that isn't
really surprising, is it?
Thanks.
--
tejun
next prev parent reply other threads:[~2007-08-03 11:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-08-02 22:42 [patch 0/5][RFC] Update network drivers to use devres Brandon Philips
2007-08-03 8:58 ` Stephen Hemminger
2007-08-03 10:26 ` Tejun Heo
2007-08-03 11:07 ` Stephen Hemminger
2007-08-03 11:33 ` Tejun Heo [this message]
2007-08-03 13:44 ` Stephen Hemminger
2007-08-03 18:23 ` Brandon Philips
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=46B31270.7090503@gmail.com \
--to=htejun@gmail.com \
--cc=brandon@ifup.org \
--cc=netdev@vger.kernel.org \
--cc=shemminger@linux-foundation.org \
--cc=teheo@suse.de \
/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).