Re: [git patches] net driver fixes

Previous thread: [PATCH] by Uwe Kleine-König on Saturday, September 13, 2008 - 1:29 pm. (1 message)

Next thread: [git patches] libata fixes by Jeff Garzik on Saturday, September 13, 2008 - 1:59 pm. (1 message)
From: Jeff Garzik
Date: Saturday, September 13, 2008 - 1:35 pm

Includes several fixes from my slice of the akpm patch pie.

Please pull from 'davem-fixes' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git davem-fixes

to receive the following updates:

 MAINTAINERS                    |    4 +++-
 drivers/net/8139too.c          |    3 ++-
 drivers/net/atl1e/atl1e_main.c |    2 --
 drivers/net/au1000_eth.c       |    3 ++-
 drivers/net/bonding/bond_alb.c |   24 ++++++++++++++++++++++++
 drivers/net/cs89x0.c           |    2 --
 drivers/net/ehea/ehea_phyp.c   |    2 +-
 drivers/net/ehea/ehea_qmr.c    |    3 ++-
 drivers/net/forcedeth.c        |   16 +++++++++++++---
 drivers/net/mlx4/alloc.c       |    1 +
 drivers/net/ne.c               |    9 ++++++++-
 drivers/net/r8169.c            |   25 ++++++++++++++-----------
 drivers/net/s2io.c             |   18 ++++++++++--------
 drivers/net/s2io.h             |    1 +
 drivers/net/skfp/pmf.c         |   29 +++++++++++------------------
 drivers/net/smc91x.c           |    3 ++-
 drivers/net/smc91x.h           |    2 ++
 drivers/net/tulip/de2104x.c    |    1 -
 drivers/net/usb/hso.c          |    9 +++++----
 drivers/net/wan/hdlc_x25.c     |    8 +++++---
 20 files changed, 106 insertions(+), 59 deletions(-)

Adrian Bunk (1):
      [netdrvr/usb] hso_create_bulk_serial_device(): fix a double free

Andrew Morton (1):
      drivers/net/mlx4/alloc.c needs mm.h

Breno Leitao (1):
      s2io: Fix enabling VLAN tag stripping at driver initialization

Chris Snook (1):
      MAINTAINERS: add Atheros maintainer for atlx

David Fries (1):
      [netdrvr] ne: Fix suspend and resume for ISA PnP cards.

Francois Romieu (1):
      r8169: fix RxMissed register access

Hannes Hering (1):
      ehea: Fix DLPAR memory handling

Krzysztof Halasa (1):
      wan/hdlc_x25.c: fix a NULL dereference

Magnus Damm (2):
      smc91x: fix nowait printout
      smc91x: SMC_IO_SHIFT platform data support for default case

Martin Gebert (1):
      [netdrvr] au1000_eth: ...
From: Ingo Oeser
Date: Sunday, September 14, 2008 - 3:51 pm

Hi Jeff,





compare_ether_addr()

Maybe fix this in a followup patch?


Best Regards

Ingo Oeser
--

From: David Miller
Date: Monday, September 15, 2008 - 12:09 pm

From: Jeff Garzik <jeff@garzik.org>

Which of these have regression and/or bugzilla entries?

This is too much and I want to see it trimmed down.

Thanks.
--

From: Jeff Garzik
Date: Monday, September 15, 2008 - 1:17 pm

What do you not want?  The two warning fixes (mlx4, cs89x0) akpm seemed
to want in 2.6.27, the 8139too change is cosmetic.  The other stuff is
critical AFAICS.

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 1:27 pm

From: Jeff Garzik <jeff@garzik.org>

I'll take another look at the queue and try to give you some
more specific feedback.

I can think of about 3 or 4 entries on the posted regression
list that might be appropriate, so I essentially expected to
see that many commits, give or take one or two.
--

From: Jeff Garzik
Date: Monday, September 15, 2008 - 1:45 pm

Don't stick strictly to that list...  We want an oops fix even if
the oops is present in 2.6.26.  We want a spinlock fix that makes a
driver work properly on SMP, even if that bug is present in 2.6.26.  We
want a build fix, etc.

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 2:14 pm

From: Jeff Garzik <jeff@garzik.org>

Look at the analysis I just posted.

Most of those changes were complete and utter CRAP.

Build warning fixes, diagnostic message tidyups.  You have
to be kidding me.

Maybe a OOPS fix for common hardware.  But not for something
like obscure WAN stuff.

But otherwise, it is in fact strict by the rules regression
list stuff.  That's all we're fixing now.

And remember it's not you who gets your head chopped off if
I try to merge in stuff that isn't appropriate now.  And Linus
has made it pretty clear what the goals of the non-merge-window
period is, and that is to fix regressions.  Otherwise the process
never comes to closure in a reasonable amount of time.
--

From: Krzysztof Halasa
Date: Monday, September 15, 2008 - 2:30 pm

FYI: the WAN bit was actually a regression, 2.6.26 doesn't have this
bug. Nevertheless generic HDLC especially with X.25 is not the
mainstream code so I guess it's probably perfectly acceptable to delay
the fix to minimize the patch flow.
-- 
Krzysztof Halasa
--

From: Thomas Bogendoerfer
Date: Monday, September 15, 2008 - 2:53 pm

thank you, but the fix I've sent (de2104x) isn't crap. The bug breaks the 
current debian-installer for Cobalt Qube1, when it tries to get an
IP address via dhcp. 

In my eyes it's a clean and good fix, but I don't care whether it appears
in 2.6.27 or 2.6.28, because debian needs to backport it to 2.6.26
anyway. But declaring bugfixes, which just fixes an very old bug, as
utter crap isn't really nice.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]
--

From: David Miller
Date: Monday, September 15, 2008 - 3:11 pm

From: tsbogend@alpha.franken.de (Thomas Bogendoerfer)

And why is this bug that has been there for YEARS is appropriate

There are a lot of clean and good fixes, but that is not the
criteria for outside-of-the merge-window submissions right now.

Linus has made this clear.
--

From: Jeff Garzik
Date: Monday, September 15, 2008 - 3:38 pm

He continues to take non-regression fixes and minor cosmetic commits
from other trees.

The basic idea is to BALANCE the lateness of -rc with the seriousness of
the problem being fixed, and the technical complexity of the fix itself.

The later the -rc, the more you need to take into account how likely it
is to create additional problems.  But did you READ the patches and
extended commit descriptions, to see if additional problems were likely?

Late -rc does not mean "no bug fixes at all except regressions" if you
look at the commits going into the kernel tree.

Late -rc does not mean putting off memory corruption fixes for 6 months.

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 3:41 pm

From: Jeff Garzik <jeff@garzik.org>

I could complain about that, but he's pushed back hard enough
on me that I don't consider that very fruitful.


Actually, since you brought this up, he's allowing this largely in
obscure subsystems and/or code he happens to be personally working on
fixes for.
--

From: Jeff Garzik
Date: Monday, September 15, 2008 - 4:25 pm

My impression was that Linus was in Wrath of God(tm) mode <grin> and he
was reacting strongly for effect not being specific.


Well, obscure subsystems like PCI

      PCI: Fix printk warnings in setup-bus.c
      PCI: Fix printk warnings in probe.c

or libata

      sata_inic162x: enable LED blinking
      ata: duplicate variable sparse warning

or ia64 (this is not a regression, but a regular bug)

	ia64: fix panic during `modprobe -r xpc'

or bugs that have existed for a long time,

	bfs: fix Lockdep warning

As I understand and observe, Linus never ever wants to create
hard-and-fast rules, in this or anything, but set a general guideline
of not putting in changes (fixes or whatever) late in the game that
have a ghost of a chance of creating further problems.  That is the
underlying issue... don't create new headaches, _especially_ for large
userbases ;-)

So, I -totally- understand your reaction, but I think this is erring
too far on the side of caution.  Oh well...

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 4:28 pm

From: Jeff Garzik <jeff@garzik.org>

Why don't you send me the trimmed down pull request for
now, and we can discuss this topic and the specific other
fixes as a follow-on thing?

I also think it matters how many fixes I batch up at once
to merge, btw.  So it's better to toss me 4 or 5 fixes
ever could of days, rather than waiting a week and sending
12 or 13.

--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 1:39 am

Such a fix is even suitable for -stable.

If something is considered suitable for -stable, but not for -rc with 
one month to go before the release of the kernel, then there's something 
wrong in the process.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Thomas Bogendoerfer
Date: Tuesday, September 16, 2008 - 2:18 am

that was my feeling as well, but if David wants to see it in the next
merge window, it's his decision. It's not a important fix as the user
base is obviuosly pretty small. And I didn't know about that bug, when
the merge windows was open, so I couldn't fix it "in time".

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]
--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 2:40 am

And if it goes in during the next merge window it can immediately go 
into 2.6.27.1 (and even 2.6.26.y if it's still maintained), since the 
fix fulfills Documentation/stable_kernel_rules.txt .


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 3:48 am

Note that my emails are not meant against David personally - he just 
seems to have gotten from pushing too much to pushing too few, with
such illogical results.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Jarek Poplawski
Date: Tuesday, September 16, 2008 - 4:43 am

Probably he can read.

Jarek P.

PS:

--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 5:15 am

I'm eagerly awaiting to slap this at Linus the next time he dares to 
directly apply a patch that only fixes a sparse warning outside of a 
merge window...  ;-)


More seriously, there's a difference between Linus' "another random 
improvement" and an "is even suitable for -stable".

I'm not reading Linus' (Cc'ed) statement the way that a patch that is 
appropriate for 2.6.27.1 is not appropriate for -rc now.

If I'm misreading Linus on this it might make sense if they'd discuss 

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Theodore Tso
Date: Tuesday, September 16, 2008 - 6:54 am

Well, remember that patches that get published for -stable do have to
go through an extra review process.  It's not true that any "obviously
correct" bug fix gets automatically published in -stable.  Sometimes
bug fixes do get rejected for -stable because they are too risky, or
require more time for testing in the -rc series before they are deemed
suitable for -stable.

Looking at 2.6.26, there is currently about 195 patches queued up,
which is certainly not all of the bugs fixed during the 2.6.27
development cycle.  Some of it is that some developers aren't as
aggressive about nominating patches for backporting to 2.6.26, and
some of it is that only the really *important* bugs get this treatment.

But, I bet that if there was a major embarassing problem where someone
abused the process by submitting an "obviously correct" patch that
ended up breaking things for a others, the end result would be that
the -stable team would start cracking down and start using more strict
criteria about what would be considered allowable for publishing in
-stable.

I'll remind people that the original rule was that new features were
only allowed in -rc1, and bug fixes in -rc1 and -rc2.  But because
that got abused, Linus is now saying that only regression bug fixes
are allowed post -rc1.  My belief is that part of the reason for this
is that Linus is a self-described soft touch, and by stating a
draconian rule, he's hoping that subsystem maintainers like David will
lay done the law, and prevent the really serious abuses (where people
starting submitting "obvious bug fixes" in -rc5 and -rc6 that caused
regressions which in turn ends up extending the release cycle).

If it's a really important bug, and it affects a huge number of users,
or it's really bad security bug, the reality is that exceptions will
be made to the rules.  But exceptions need to remain exceptions for
extraordinary situations, not everyday occurrences.  And of course, if
the bug does affect a huge number of users, someone ...
From: Arjan van de Ven
Date: Tuesday, September 16, 2008 - 7:02 am

On Tue, 16 Sep 2008 09:54:08 -0400

also note that Linus said "regression or on the kerneloops list";
if it has any kind of backtrace, it'll be there if it's a common
problem that hits many users.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 7:34 am

Thomas' patch [1] will fixes a bug of a kind that will likely never 
make it to your list.

And the same also e.g. goes for bugs where your machine is completely 
dead (no SysRq possible) with nothing in the logs.

The kerneloops lists are quite valuable, but they can never cover all 
classes of fatal bugs.

cu
Adrian

[1] http://article.gmane.org/gmane.linux.network/105810

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Arjan van de Ven
Date: Tuesday, September 16, 2008 - 7:48 am

On Tue, 16 Sep 2008 17:34:10 +0300

not sure; just we need to catch doing pci_disable_device on a
non-enabled device as a WARN_ON.

and the patch looks quite wrong, the real answer should be to do the

that's being worked on, by storing the crash data in some non-volatile

if you want perfection, you're not going to get it.

"Perfect is the enemy of good"

If you want to help make things better (as opposed to perfect), you're
very welcome to help out, be it with suggestions on how to improve
or with actual code. 



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--

From: Thomas Bogendoerfer
Date: Tuesday, September 16, 2008 - 8:21 am

the pci_disable_device() is done on an enabled device, so your

I thought about the solution, but the pci device is completely setup
and enabled in the init function so disabling it at that point and
enabling it in open() again sounds silly to me. pci_enable_enable/disable
is already done in .init/.remove and .suspend/.resume, so it looked
more obviuos to not let close() do the pci_disable_device().

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]
--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 8:43 am

The kerneloops list is helpful, and improving it is great, but it will 

You miss my point:

We agree that the kerneloops list is not perfect.

And usually the bugs with a trace that go into the list are the
ones that are easier to debug and fix, while the others might be
much harder to debug.

If one takes Linus' "simple rule of thumb" literally then a hard to 
debug bug without any messages anywhere mustn't be fixed outside of
a merge window. This literal reading is what doesn't make sense.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Adrian Bunk
Date: Tuesday, September 16, 2008 - 7:46 am

Among the patches David rejected based on what Linus said was
Thomas' patch [1].

It is not a "really important bug" and does not affect "huge number
of users".

But it passes Documentation/stable_kernel_rules.txt, and can we agree 
that if it gets submitted for 2.6.27.1 it will likely pass review and 
get applied?


cu
Adrian

[1] http://article.gmane.org/gmane.linux.network/105810

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: David Miller
Date: Monday, September 15, 2008 - 2:11 pm

From: David Miller <davem@davemloft.net>

	Thomas Bogendoerfer (1):
	      tulip: Fix dead 21041 ethernet after ifconfig down

Not a regression, that problem has been there forever.

	Adrian Bunk (1):
	      [netdrvr/usb] hso_create_bulk_serial_device(): fix a double free

This is a new driver in 2.6.27, so OK.

	Andrew Morton (1):
	      drivers/net/mlx4/alloc.c needs mm.h

Build bustage, so ok.

	Breno Leitao (1):
	      s2io: Fix enabling VLAN tag stripping at driver initialization

Not a 2.6.27 regression, 2.6.26 has this problem too.

	Chris Snook (1):
	      MAINTAINERS: add Atheros maintainer for atlx

Doc change, so fine I guess.

	David Fries (1):
	      [netdrvr] ne: Fix suspend and resume for ISA PnP cards.

Definitely not a 2.6.27 regression.

	Francois Romieu (1):
	      r8169: fix RxMissed register access

Has a bugzilla entry, etc., so OK.

	Hannes Hering (1):
	      ehea: Fix DLPAR memory handling

No kernel bugzilla, no regression list entry, no way.

	Krzysztof Halasa (1):
	      wan/hdlc_x25.c: fix a NULL dereference

Not a 2.6.27 regression, this problem has been there forever.

	Magnus Damm (2):
	      smc91x: fix nowait printout

This one is good but the commit message needs to be fixed up.
It references a raw SHA1 ID but does not give the commit message
header line as well, which is a requirement.

      smc91x: SMC_IO_SHIFT platform data support for default case

Ok, but needs commit header line text for referenced commit.

	Martin Gebert (1):
	      [netdrvr] au1000_eth: Spinlock initialisation fix

This problem has existed forever, not a 2.6.27 regression,
and therefore not appropriate.

	Mats Erik Andersson (1):
	      8139too: [cosmetic] fix incorrect register for flash-rom

Cosmetic, indeed.  Not appropriate for 2.6.27

	Matthew Wilcox (1):
	      [netdrvr] atl1e: Don't take the mdio_lock in atl1e_probe

This is just lockep noise, and definitely not a 2.6.27 regression
as far as I can tell.  So this one has ...
From: Jeff Garzik
Date: Monday, September 15, 2008 - 2:24 pm

So, my takeaway from this is...

1) Creating a bugzilla entry magically makes a bug fix acceptable?

2) We no longer want "this kills the driver" fixes?

I disagree with that logic, and I seriously doubt Linus wants to turn
away serious fixes to serious problems.  Many of these are clearly
needed, just read the extended patch description and the patch itself.

You just rejected patches that (a) fixed dead ethernet [de2104x], (b) fixed an
oops [WAN], and (c) fixed memory corruption [ehea].

I feel like I have just stepped into Odd World, if you don't want fixes
as serious as these.

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 3:07 pm

From: Jeff Garzik <jeff@garzik.org>

It adds more credence to the bug.  Because it's usually reported

I don't take that away from what I said.

Why is it so important to fix something "RIGHT NOW" that
has existed forever?  Where were these fixes during the

He wants the time outside of the merge window to work on fixing
regressions and exploitable problems.  This is so that the process

And it's getting submitted now, because?!?



Again, it's been there for quite some time, why is it "now"
all of sudden now important to integrate this fix?
--

From: Jeff Garzik
Date: Monday, September 15, 2008 - 3:34 pm

Because users should not be forced to wait 3 months for fixes, for
admittedly serious bugs, which are ready for them right now.  Small,
self-contained, tested, obvious fixes.

There is just something horribly wrong if you are not taking two-line
fixes for memory corruption, even if the problem existed since
kernel 1.2.13.

	July 13:	2.6.26 released, merge window opens
	July 29:	2.6.27-rc1 released, merge window closes
	Sept 20?	2.6.27 release (wild guess)
	Dec 25?		2.6.28 release (wild guess)

So if a small, easily-mergable fix for a major bug like an oops
appears on July 30, being strict to your plan has this fix waiting
until almost 2009 (2.6.28 release), a wait of over five months.

That is not serving Linux users, our primary customers.

Complaining about the 8139too change, I can see that (even though
Linus takes that stuff via libata all the time).  But about memory
corruption and oopses?  Come on.

	Jeff



--

From: David Miller
Date: Monday, September 15, 2008 - 3:39 pm

From: Jeff Garzik <jeff@garzik.org>

I personally partly agree with you, but Linus put his foot down and
I'm implementing the stated policy by doing the same.
--

From: Simon Horman
Date: Monday, September 15, 2008 - 7:01 pm

On Mon, Sep 15, 2008 at 02:11:59PM -0700, David Miller wrote:


[snip]

Hi Dave,

can you confirm that the commit message requirement stands even
if the SHA1 ID references Linus' tree? I personally think that is
reasonable, I just want to confirm that is the requirement.

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

--

From: David Miller
Date: Saturday, September 20, 2008 - 12:24 am

From: Simon Horman <horms@verge.net.au>

Yes, because the commit messages will propagate back usually
verbatim for changes backported into the -stable branches.
--

Previous thread: [PATCH] by Uwe Kleine-König on Saturday, September 13, 2008 - 1:29 pm. (1 message)

Next thread: [git patches] libata fixes by Jeff Garzik on Saturday, September 13, 2008 - 1:59 pm. (1 message)