Re: Network: convert network devices to use struct device instead of class_device

Re: Network: convert network devices to use struct device instead of class_device

[Jeff Garzik]

Linux Kernel Mailing List wrote:
> Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> Author:     Greg Kroah-Hartman <gregkh@suse.de>
> AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> CommitDate: Wed Feb 7 10:37:11 2007 -0800
> 
>     Network: convert network devices to use struct device instead of class_device
>     
>     This lets the network core have the ability to handle suspend/resume
>     issues, if it wants to.
>     
>     Thanks to Frederik Deweerdt <frederik.deweerdt@gmail.com> for the arm
>     driver fixes.
>     
>     Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c |   33 ++--
>  drivers/infiniband/ulp/ipoib/ipoib_vlan.c |   11 +-
>  drivers/net/arm/at91_ether.c              |    2 +-
>  drivers/net/arm/etherh.c                  |    2 +-
>  drivers/net/bonding/bond_sysfs.c          |  287 +++++++++++++++++-----------
>  drivers/net/iseries_veth.c                |    2 +-
>  drivers/net/macb.c                        |   36 ++--
>  drivers/net/smc911x.c                     |    2 +-
>  drivers/net/smc91x.c                      |    2 +-
>  drivers/net/wireless/hostap/hostap_main.c |    2 +-
>  drivers/net/wireless/orinoco.c            |    4 +-
>  drivers/net/wireless/orinoco_cs.c         |    2 +-
>  drivers/net/wireless/spectrum_cs.c        |    2 +-
>  include/linux/netdevice.h                 |    5 +-
>  net/bridge/br_if.c                        |    2 +-
>  net/bridge/br_sysfs_br.c                  |  234 +++++++++++++-----------
>  net/bridge/br_sysfs_if.c                  |    2 +-
>  net/core/dev.c                            |    6 +-
>  net/core/net-sysfs.c                      |  175 ++++++++++--------
>  net/core/skbuff.c                         |    2 +-
>  20 files changed, 451 insertions(+), 362 deletions(-)

It's highly disappointing that this was never discussed on 
netdev@vger.kernel.org, or even CC'd to me.

	Jeff

Re: Network: convert network devices to use struct device instead of class_device

[Stephen Hemminger]

On Thu, 08 Feb 2007 07:43:18 -0500
Jeff Garzik <jeff@garzik.org> wrote:

> Linux Kernel Mailing List wrote:
> > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> > 
> >     Network: convert network devices to use struct device instead of class_device
> >     
> >     This lets the network core have the ability to handle suspend/resume
> >     issues, if it wants to.

It fixes a non-problem. I would like to see the network core suspend/resume
proposal as well. Last time I examined doing network core suspend help,
the problem was that the physical device suspend was called before the
class device. It is not clear how this change would help.

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> On Thu, 08 Feb 2007 07:43:18 -0500
> Jeff Garzik <jeff@garzik.org> wrote:
>
> > Linux Kernel Mailing List wrote:
> > > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> > > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> > > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> > > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> > > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> > > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> > > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> > >
> > >     Network: convert network devices to use struct device instead of class_device
> > >
> > >     This lets the network core have the ability to handle suspend/resume
> > >     issues, if it wants to.
>
> It fixes a non-problem. I would like to see the network core suspend/resume
> proposal as well. Last time I examined doing network core suspend help,
> the problem was that the physical device suspend was called before the
> class device. It is not clear how this change would help.

If physical devices are registered before class devices then when
suspending class devices are naturally suspended first. It is still
not clear to me why we need to convert everythign to struct device, I
believe I've shown (with patches) that it is possible to integrate
struct class_device into PM framework and avoid reshuffling half of
the kernel code.

-- 
Dmitry

Re: Network: convert network devices to use struct device instead of class_device

[Greg KH]

On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
> On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> >On Thu, 08 Feb 2007 07:43:18 -0500
> >Jeff Garzik <jeff@garzik.org> wrote:
> >
> >> Linux Kernel Mailing List wrote:
> >> > Gitweb:     
> >http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> >> > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> >> > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> >> > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> >> > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> >> > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> >> > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> >> >
> >> >     Network: convert network devices to use struct device instead of 
> >class_device
> >> >
> >> >     This lets the network core have the ability to handle 
> >suspend/resume
> >> >     issues, if it wants to.
> >
> >It fixes a non-problem. I would like to see the network core suspend/resume
> >proposal as well. Last time I examined doing network core suspend help,
> >the problem was that the physical device suspend was called before the
> >class device. It is not clear how this change would help.
> 
> If physical devices are registered before class devices then when
> suspending class devices are naturally suspended first. It is still
> not clear to me why we need to convert everythign to struct device, I
> believe I've shown (with patches) that it is possible to integrate
> struct class_device into PM framework and avoid reshuffling half of
> the kernel code.

I don't want to have two separate device trees in the kernel (well, one
big device tree and a bunch of little class_device trees.)  The code
duplication in the class_device code is just too much, and I get
questions all the time as to what the differences are.

With these slow and gradual changes, we are getting a true, unified,
device tree, and it will reduce the amount of code and complexity we
need to maintain and fix in the driver core itself.

And it should also alow for proper power management functionality, using
the changes that Linus put into the driver core about 8 months ago.

Don't worry, I have input patches queued up next for you Dmitry :)

thanks,

greg k-h

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On Thursday 08 February 2007 19:56, Greg KH wrote:
> On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
> > On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > >On Thu, 08 Feb 2007 07:43:18 -0500
> > >Jeff Garzik <jeff@garzik.org> wrote:
> > >
> > >> Linux Kernel Mailing List wrote:
> > >> > Gitweb:     
> > >http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> > >> > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> > >> > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> > >> > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> > >> > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> > >> > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> > >> > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> > >> >
> > >> >     Network: convert network devices to use struct device instead of 
> > >class_device
> > >> >
> > >> >     This lets the network core have the ability to handle 
> > >suspend/resume
> > >> >     issues, if it wants to.
> > >
> > >It fixes a non-problem. I would like to see the network core suspend/resume
> > >proposal as well. Last time I examined doing network core suspend help,
> > >the problem was that the physical device suspend was called before the
> > >class device. It is not clear how this change would help.
> > 
> > If physical devices are registered before class devices then when
> > suspending class devices are naturally suspended first. It is still
> > not clear to me why we need to convert everythign to struct device, I
> > believe I've shown (with patches) that it is possible to integrate
> > struct class_device into PM framework and avoid reshuffling half of
> > the kernel code.
> 
> I don't want to have two separate device trees in the kernel (well, one
> big device tree and a bunch of little class_device trees.)  The code
> duplication in the class_device code is just too much, and I get
> questions all the time as to what the differences are.
>

While duplication of code is a real concern my worry is constant fattening
of struct device. For example most physical devices do not interface
directly with userspace but every single one of them now has dev_t.
Former class_devices do not need suspend/resume early framework either.
And so on, and so forth.
 
> With these slow and gradual changes, we are getting a true, unified,
> device tree, and it will reduce the amount of code and complexity we
> need to maintain and fix in the driver core itself.
> 
> And it should also alow for proper power management functionality, using
> the changes that Linus put into the driver core about 8 months ago.
> 
> Don't worry, I have input patches queued up next for you Dmitry :)
> 

;)

-- 
Dmitry

Re: Network: convert network devices to use struct device instead of class_device

[Kay Sievers]

On Thu, 2007-02-08 at 22:59 -0500, Dmitry Torokhov wrote:
> On Thursday 08 February 2007 19:56, Greg KH wrote:
> > On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
> > > On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > >On Thu, 08 Feb 2007 07:43:18 -0500

> > > >> >     Network: convert network devices to use struct device instead of 
> > > >class_device
> > > >> >
> > > >> >     This lets the network core have the ability to handle 
> > > >suspend/resume
> > > >> >     issues, if it wants to.
> > > >
> > > >It fixes a non-problem. I would like to see the network core suspend/resume
> > > >proposal as well. Last time I examined doing network core suspend help,
> > > >the problem was that the physical device suspend was called before the
> > > >class device. It is not clear how this change would help.
> > > 
> > > If physical devices are registered before class devices then when
> > > suspending class devices are naturally suspended first. It is still
> > > not clear to me why we need to convert everythign to struct device, I
> > > believe I've shown (with patches) that it is possible to integrate
> > > struct class_device into PM framework and avoid reshuffling half of
> > > the kernel code.
> > 
> > I don't want to have two separate device trees in the kernel (well, one
> > big device tree and a bunch of little class_device trees.)  The code
> > duplication in the class_device code is just too much, and I get
> > questions all the time as to what the differences are.
> >
> 
> While duplication of code is a real concern my worry is constant fattening
> of struct device. For example most physical devices do not interface
> directly with userspace but every single one of them now has dev_t.
> Former class_devices do not need suspend/resume early framework either.
> And so on, and so forth.

The dev_t is a good example for the mess we try to fix here. Not having
a dev_t for "devices" lead to the creation of a lot of otherwise
completely useless "class devices" which are just a total pain to
interpret, and follow the events they create, from userspace.

Things like the scsi_device devices, usb_device devices, ... just exist,
because only this type of devices was allowed to pass information for
device nodes to userspace.

Thanks,
Kay

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/9/07, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, 2007-02-08 at 22:59 -0500, Dmitry Torokhov wrote:
> > On Thursday 08 February 2007 19:56, Greg KH wrote:
> > > On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
> > > > On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > > >On Thu, 08 Feb 2007 07:43:18 -0500
>
> > > > >> >     Network: convert network devices to use struct device instead of
> > > > >class_device
> > > > >> >
> > > > >> >     This lets the network core have the ability to handle
> > > > >suspend/resume
> > > > >> >     issues, if it wants to.
> > > > >
> > > > >It fixes a non-problem. I would like to see the network core suspend/resume
> > > > >proposal as well. Last time I examined doing network core suspend help,
> > > > >the problem was that the physical device suspend was called before the
> > > > >class device. It is not clear how this change would help.
> > > >
> > > > If physical devices are registered before class devices then when
> > > > suspending class devices are naturally suspended first. It is still
> > > > not clear to me why we need to convert everythign to struct device, I
> > > > believe I've shown (with patches) that it is possible to integrate
> > > > struct class_device into PM framework and avoid reshuffling half of
> > > > the kernel code.
> > >
> > > I don't want to have two separate device trees in the kernel (well, one
> > > big device tree and a bunch of little class_device trees.)  The code
> > > duplication in the class_device code is just too much, and I get
> > > questions all the time as to what the differences are.
> > >
> >
> > While duplication of code is a real concern my worry is constant fattening
> > of struct device. For example most physical devices do not interface
> > directly with userspace but every single one of them now has dev_t.
> > Former class_devices do not need suspend/resume early framework either.
> > And so on, and so forth.
>
> The dev_t is a good example for the mess we try to fix here. Not having
> a dev_t for "devices" lead to the creation of a lot of otherwise
> completely useless "class devices" which are just a total pain to
> interpret, and follow the events they create, from userspace.
>
> Things like the scsi_device devices, usb_device devices, ... just exist,
> because only this type of devices was allowed to pass information for
> device nodes to userspace.
>

I admit I do not know scsi stack but I would expect that the only
things that need dev_t there would be sd, sr and sg interfaces. As
such they are separate entities and "deserve" their own structures no
mater what.

I can bet that number of real devices that need dev_t is smaller than
number of virtual devices that do not need full power management:

PCI cards, ACPI tree, etc, etc - hardware devices interfacing with
other parts of the kernel, not userspace directly.

NET, input, tty, etc - no need to suspend late/resume early

And, btw, having separate device and struct device does not prevent
exporting them as a unified sysfs tree and is in fact pretty easy to
do (I believe I posted patches to do that as well).

-- 
Dmitry

Re: Network: convert network devices to use struct device instead of class_device

[Greg KH]

On Thu, Feb 08, 2007 at 10:59:46PM -0500, Dmitry Torokhov wrote:
> On Thursday 08 February 2007 19:56, Greg KH wrote:
> > On Thu, Feb 08, 2007 at 12:29:12PM -0500, Dmitry Torokhov wrote:
> > > On 2/8/07, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > >On Thu, 08 Feb 2007 07:43:18 -0500
> > > >Jeff Garzik <jeff@garzik.org> wrote:
> > > >
> > > >> Linux Kernel Mailing List wrote:
> > > >> > Gitweb:     
> > > >http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> > > >> > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> > > >> > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> > > >> > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> > > >> > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> > > >> > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> > > >> > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> > > >> >
> > > >> >     Network: convert network devices to use struct device instead of 
> > > >class_device
> > > >> >
> > > >> >     This lets the network core have the ability to handle 
> > > >suspend/resume
> > > >> >     issues, if it wants to.
> > > >
> > > >It fixes a non-problem. I would like to see the network core suspend/resume
> > > >proposal as well. Last time I examined doing network core suspend help,
> > > >the problem was that the physical device suspend was called before the
> > > >class device. It is not clear how this change would help.
> > > 
> > > If physical devices are registered before class devices then when
> > > suspending class devices are naturally suspended first. It is still
> > > not clear to me why we need to convert everythign to struct device, I
> > > believe I've shown (with patches) that it is possible to integrate
> > > struct class_device into PM framework and avoid reshuffling half of
> > > the kernel code.
> > 
> > I don't want to have two separate device trees in the kernel (well, one
> > big device tree and a bunch of little class_device trees.)  The code
> > duplication in the class_device code is just too much, and I get
> > questions all the time as to what the differences are.
> >
> 
> While duplication of code is a real concern my worry is constant fattening
> of struct device. For example most physical devices do not interface
> directly with userspace but every single one of them now has dev_t.
> Former class_devices do not need suspend/resume early framework either.
> And so on, and so forth.

The memory requirements of 'struct device' is trivial in the overall
system requirements.

Now this doesn't mean we need to go crazy with adding everything and the
kitchen sink to the structure, but adding a dev_t is not going to make
any difference to anyone.

thanks,

greg k-h

Re: Network: convert network devices to use struct device instead of class_device

[Dmitry Torokhov]

On 2/8/07, Greg KH <greg@kroah.com> wrote:
>
> And it should also alow for proper power management functionality, using
> the changes that Linus put into the driver core about 8 months ago.
>
> Don't worry, I have input patches queued up next for you Dmitry :)
>

Greg,

Could you please forward me these patches for input core - I am going
to rearrange some code and don't want too many clashes so I'd like to
apply yours to my private three and work from there...

Thanks!

-- 
Dmitry

Re: Network: convert network devices to use struct device instead of class_device

[Greg KH]

On Thu, Feb 08, 2007 at 07:41:30AM -0800, Stephen Hemminger wrote:
> On Thu, 08 Feb 2007 07:43:18 -0500
> Jeff Garzik <jeff@garzik.org> wrote:
> 
> > Linux Kernel Mailing List wrote:
> > > Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> > > Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> > > Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> > > Author:     Greg Kroah-Hartman <gregkh@suse.de>
> > > AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> > > Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> > > CommitDate: Wed Feb 7 10:37:11 2007 -0800
> > > 
> > >     Network: convert network devices to use struct device instead of class_device
> > >     
> > >     This lets the network core have the ability to handle suspend/resume
> > >     issues, if it wants to.
> 
> It fixes a non-problem. I would like to see the network core suspend/resume
> proposal as well. Last time I examined doing network core suspend help,
> the problem was that the physical device suspend was called before the
> class device. It is not clear how this change would help.

Now the real device of the network device (ethX and such) will be called
first.  If not, something's wrong...

thanks,

greg k-h

Re: Network: convert network devices to use struct device instead of class_device

[Greg KH]

On Thu, Feb 08, 2007 at 07:43:18AM -0500, Jeff Garzik wrote:
> Linux Kernel Mailing List wrote:
> >Gitweb:     
> >http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=43cb76d91ee85f579a69d42bc8efc08bac560278
> >Commit:     43cb76d91ee85f579a69d42bc8efc08bac560278
> >Parent:     2943ecf2ed32632473c06f1975db47a7aa98c10f
> >Author:     Greg Kroah-Hartman <gregkh@suse.de>
> >AuthorDate: Tue Apr 9 12:14:34 2002 -0700
> >Committer:  Greg Kroah-Hartman <gregkh@suse.de>
> >CommitDate: Wed Feb 7 10:37:11 2007 -0800
> >
> >    Network: convert network devices to use struct device instead of 
> >    class_device
> >    
> >    This lets the network core have the ability to handle suspend/resume
> >    issues, if it wants to.
> >    
> >    Thanks to Frederik Deweerdt <frederik.deweerdt@gmail.com> for the arm
> >    driver fixes.
> >    
> >    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> >---
> > drivers/infiniband/ulp/ipoib/ipoib_main.c |   33 ++--
> > drivers/infiniband/ulp/ipoib/ipoib_vlan.c |   11 +-
> > drivers/net/arm/at91_ether.c              |    2 +-
> > drivers/net/arm/etherh.c                  |    2 +-
> > drivers/net/bonding/bond_sysfs.c          |  287 
> > +++++++++++++++++-----------
> > drivers/net/iseries_veth.c                |    2 +-
> > drivers/net/macb.c                        |   36 ++--
> > drivers/net/smc911x.c                     |    2 +-
> > drivers/net/smc91x.c                      |    2 +-
> > drivers/net/wireless/hostap/hostap_main.c |    2 +-
> > drivers/net/wireless/orinoco.c            |    4 +-
> > drivers/net/wireless/orinoco_cs.c         |    2 +-
> > drivers/net/wireless/spectrum_cs.c        |    2 +-
> > include/linux/netdevice.h                 |    5 +-
> > net/bridge/br_if.c                        |    2 +-
> > net/bridge/br_sysfs_br.c                  |  234 +++++++++++++-----------
> > net/bridge/br_sysfs_if.c                  |    2 +-
> > net/core/dev.c                            |    6 +-
> > net/core/net-sysfs.c                      |  175 ++++++++++--------
> > net/core/skbuff.c                         |    2 +-
> > 20 files changed, 451 insertions(+), 362 deletions(-)
> 
> It's highly disappointing that this was never discussed on 
> netdev@vger.kernel.org, or even CC'd to me.

It was CCed to David Miller and you and netdev a long time ago (like 8-9
months ago.)  It's been in the -mm tree since then and discussed on lkml
a few times too.

As I got David's ack for it, and he said to push it through my tree, I
didn't think any more of it.

My apologies, I should have consulted you again before doing it.

thanks,

greg k-h