* [git patch] urgent e1000 fix
@ 2005-06-23 9:24 Jeff Garzik
2005-06-23 21:04 ` David Lang
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Garzik @ 2005-06-23 9:24 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: Linux Kernel, Netdev List
[-- Attachment #1: Type: text/plain, Size: 174 bytes --]
Please pull from 'misc-fixes' branch of
rsync://rsync.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
to obtain the spinlock fix described in the attached text.
[-- Attachment #2: netdev-2.6.txt --]
[-- Type: text/plain, Size: 495 bytes --]
drivers/net/e1000/e1000_main.c | 1 +
1 files changed, 1 insertion(+)
Mitch Williams:
e1000: fix spinlock bug
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2307,6 +2307,7 @@ e1000_xmit_frame(struct sk_buff *skb, st
tso = e1000_tso(adapter, skb);
if (tso < 0) {
dev_kfree_skb_any(skb);
+ spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_OK;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 9:24 [git patch] urgent e1000 fix Jeff Garzik
@ 2005-06-23 21:04 ` David Lang
2005-06-23 21:19 ` Jeff Garzik
0 siblings, 1 reply; 13+ messages in thread
From: David Lang @ 2005-06-23 21:04 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, Linus Torvalds, Linux Kernel, Netdev List
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1528 bytes --]
hmm, I know I'm not that experianced with patch, but when I saved this to
a file and did patch -p1 <file the hunk was rejected, the reject file is
saying
***************
*** 2307,2312 ****
tso = e1000_tso(adapter, skb);
if (tso < 0) {
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
--- 2307,2313 ----
tso = e1000_tso(adapter, skb);
if (tso < 0) {
dev_kfree_skb_any(skb);
+ spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_OK;
}
I manually put the line in and am compiling it now to test it, but is
someone could take a few seconds and teach me what I did wrong it would be
appriciated.
David Lang
On Thu, 23 Jun 2005, Jeff Garzik wrote:
> Date: Thu, 23 Jun 2005 05:24:05 -0400
> From: Jeff Garzik <jgarzik@pobox.com>
> To: Andrew Morton <akpm@osdl.org>, Linus Torvalds <torvalds@osdl.org>
> Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
> Netdev List <netdev@vger.kernel.org>
> Subject: [git patch] urgent e1000 fix
>
> Please pull from 'misc-fixes' branch of
> rsync://rsync.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
>
> to obtain the spinlock fix described in the attached text.
>
>
>
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare
[-- Attachment #2: Type: TEXT/PLAIN, Size: 495 bytes --]
drivers/net/e1000/e1000_main.c | 1 +
1 files changed, 1 insertion(+)
Mitch Williams:
e1000: fix spinlock bug
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2307,6 +2307,7 @@ e1000_xmit_frame(struct sk_buff *skb, st
tso = e1000_tso(adapter, skb);
if (tso < 0) {
dev_kfree_skb_any(skb);
+ spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_OK;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 21:04 ` David Lang
@ 2005-06-23 21:19 ` Jeff Garzik
2005-06-23 22:08 ` Linus Torvalds
2005-06-23 23:20 ` David Lang
0 siblings, 2 replies; 13+ messages in thread
From: Jeff Garzik @ 2005-06-23 21:19 UTC (permalink / raw)
To: David Lang; +Cc: Andrew Morton, Linus Torvalds, Linux Kernel, Netdev List
David Lang wrote:
> hmm, I know I'm not that experianced with patch, but when I saved this
> to a file and did patch -p1 <file the hunk was rejected, the reject file
> is saying
It's probably the whitespace thing that Linus's git-apply gadget was
complaining about.
I'm terribly surprising, though, since my patch(1) applied the diff just
fine.
<shrug>
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 21:19 ` Jeff Garzik
@ 2005-06-23 22:08 ` Linus Torvalds
2005-06-23 23:10 ` Jeff Garzik
2005-06-23 23:20 ` David Lang
1 sibling, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2005-06-23 22:08 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Lang, Andrew Morton, Linux Kernel, Netdev List
On Thu, 23 Jun 2005, Jeff Garzik wrote:
>
> It's probably the whitespace thing that Linus's git-apply gadget was
> complaining about.
>
> I'm terribly surprising, though, since my patch(1) applied the diff just
> fine.
I could easily make git-apply accept empty lines as if they had a single
space on it. What I find surprising is that "patch" allows that kind of
whitespace corruption by default, and silently. Usually you have to give
it the "-l" flag to make it ignore whitespace differences..
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 22:08 ` Linus Torvalds
@ 2005-06-23 23:10 ` Jeff Garzik
2005-06-23 23:33 ` Linus Torvalds
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Garzik @ 2005-06-23 23:10 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Lang, Andrew Morton, Linux Kernel, Netdev List
Linus Torvalds wrote:
>
> On Thu, 23 Jun 2005, Jeff Garzik wrote:
>
>>It's probably the whitespace thing that Linus's git-apply gadget was
>>complaining about.
>>
>>I'm terribly surprising, though, since my patch(1) applied the diff just
>>fine.
>
>
> I could easily make git-apply accept empty lines as if they had a single
> space on it. What I find surprising is that "patch" allows that kind of
> whitespace corruption by default, and silently. Usually you have to give
> it the "-l" flag to make it ignore whitespace differences..
patch(1) is a huge collection of heuristics like this, even without
'-l', so I'm not surprised that it worked.
Does git-apply support patches with "fuzz", out of curiosity?
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 21:19 ` Jeff Garzik
2005-06-23 22:08 ` Linus Torvalds
@ 2005-06-23 23:20 ` David Lang
1 sibling, 0 replies; 13+ messages in thread
From: David Lang @ 2005-06-23 23:20 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, Linus Torvalds, Linux Kernel, Netdev List
This does not solve the problem that I reported last thursday where the
fourth port of a e1000 quad card doesn't work under SMP (I don't know if
it was supposed to, but since it was a locking fix I had hopes).
here's the symptom I am seeing
happy1-p:~# ifconfig eth11 192.168.255.1
SIOCSIFFLAGS: Invalid argument
happy1-p:~# netstat -nr
Kernel IP routing table
Destination Gateway Genmask Flags MSS Window irtt Iface
172.20.252.0 0.0.0.0 255.255.252.0 U 0 0 0 eth1
10.201.0.0 0.0.0.0 255.255.0.0 U 0 0 0 eth0
0.0.0.0 10.201.0.2 0.0.0.0 UG 0 0 0 eth0
happy1-p:~# ifconfig eth11
eth11 Link encap:Ethernet HWaddr 00:04:23:B4:BB:97
inet addr:192.168.255.1 Bcast:192.168.255.255 Mask:255.255.255.0
BROADCAST MULTICAST MTU:1500 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:0 (0.0 b) TX bytes:0 (0.0 b)
Base address:0x60c0 Memory:fe460000-fe480000
userspace is debian woody
David Lang
On Thu, 23 Jun 2005, Jeff Garzik wrote:
> David Lang wrote:
>> hmm, I know I'm not that experianced with patch, but when I saved this to a
>> file and did patch -p1 <file the hunk was rejected, the reject file is
>> saying
>
> It's probably the whitespace thing that Linus's git-apply gadget was
> complaining about.
>
> I'm terribly surprising, though, since my patch(1) applied the diff just
> fine.
>
> <shrug>
>
> Jeff
>
>
>
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 23:10 ` Jeff Garzik
@ 2005-06-23 23:33 ` Linus Torvalds
2005-06-24 6:49 ` Denis Vlasenko
0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2005-06-23 23:33 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Lang, Andrew Morton, Linux Kernel, Netdev List
On Thu, 23 Jun 2005, Jeff Garzik wrote:
>
> patch(1) is a huge collection of heuristics like this, even without
> '-l', so I'm not surprised that it worked.
>
> Does git-apply support patches with "fuzz", out of curiosity?
No, git is strictly "zero fuzz". So was my "dotest" script even before
switching it to "git-apply", btw (ie I explicitly had a "--fuzz=0" there
when using GNU patch).
Now, I may have to reconsider the strictness if it causes lots of
problems, and it's not like I hate fuzz (or ignoring whitespace) with a
passion. I'd just rather try to be as strict as possible at least
initially.
Your special case of a corrupted patch where a single space turned into an
empty line is actually one case I considered allowing, just because the
fuzz there wouldn't be in the data itself, only in the first character
that just tells what kind of line it is (new, deleted or unmodified), and
just saying "empty means unmodified" doesn't really introduce any
possibility of ambiguity.
So let's see what happens. Tell me if you see more of this particular kind
of corruption, and I'll just make "git-apply" work around it. I don't
think _one_ patch is much of an indication in any direction, but two or
three might convince me to just relax the checks a bit.
To actually allow real fuzz or to allow real whitespace differences in the
patch data itself is a _much_ bigger issue than this trivial patch
corruption, and I'd prefer to avoid going there if at all possible.
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-23 23:33 ` Linus Torvalds
@ 2005-06-24 6:49 ` Denis Vlasenko
2005-06-24 8:22 ` Keith Owens
2005-06-24 15:11 ` Horst von Brand
0 siblings, 2 replies; 13+ messages in thread
From: Denis Vlasenko @ 2005-06-24 6:49 UTC (permalink / raw)
To: Linus Torvalds, Jeff Garzik
Cc: David Lang, Andrew Morton, Linux Kernel, Netdev List
On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> To actually allow real fuzz or to allow real whitespace differences in the
> patch data itself is a _much_ bigger issue than this trivial patch
> corruption, and I'd prefer to avoid going there if at all possible.
How about automatic stripping of _trailing_ whitespace on all incoming
patches? IIRC no file type (C, sh, Makefile, you name it) depends on
conservation of it, thus it's 100% safe.
--
vda
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-24 6:49 ` Denis Vlasenko
@ 2005-06-24 8:22 ` Keith Owens
2005-06-24 8:51 ` Linus Torvalds
2005-06-24 15:11 ` Horst von Brand
1 sibling, 1 reply; 13+ messages in thread
From: Keith Owens @ 2005-06-24 8:22 UTC (permalink / raw)
To: Denis Vlasenko
Cc: Linus Torvalds, Jeff Garzik, David Lang, Andrew Morton,
Linux Kernel, Netdev List
On Fri, 24 Jun 2005 09:49:05 +0300,
Denis Vlasenko <vda@ilport.com.ua> wrote:
>On Friday 24 June 2005 02:33, Linus Torvalds wrote:
>> To actually allow real fuzz or to allow real whitespace differences in the
>> patch data itself is a _much_ bigger issue than this trivial patch
>> corruption, and I'd prefer to avoid going there if at all possible.
>
>How about automatic stripping of _trailing_ whitespace on all incoming
>patches? IIRC no file type (C, sh, Makefile, you name it) depends on
>conservation of it, thus it's 100% safe.
One (admittedly rare) case - adding a text file that contains an
embedded patch, so you have a patch that includes a patch. This is
sometimes done in Documentation files when an external file has to be
changed. In embedded patch, empty lines are converted to a single
space, which then appears as trailing whitespace. Not sure if that is
a big enough reason not to strip whitespace.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-24 8:22 ` Keith Owens
@ 2005-06-24 8:51 ` Linus Torvalds
2005-06-24 10:14 ` Tomasz Kłoczko
0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2005-06-24 8:51 UTC (permalink / raw)
To: Keith Owens
Cc: Denis Vlasenko, Jeff Garzik, David Lang, Andrew Morton,
Linux Kernel, Netdev List
On Fri, 24 Jun 2005, Keith Owens wrote:
> On Fri, 24 Jun 2005 09:49:05 +0300,
> Denis Vlasenko <vda@ilport.com.ua> wrote:
> >On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> >> To actually allow real fuzz or to allow real whitespace differences in the
> >> patch data itself is a _much_ bigger issue than this trivial patch
> >> corruption, and I'd prefer to avoid going there if at all possible.
> >
> >How about automatic stripping of _trailing_ whitespace on all incoming
> >patches? IIRC no file type (C, sh, Makefile, you name it) depends on
> >conservation of it, thus it's 100% safe.
>
> One (admittedly rare) case - adding a text file that contains an
> embedded patch, so you have a patch that includes a patch. This is
> sometimes done in Documentation files when an external file has to be
> changed. In embedded patch, empty lines are converted to a single
> space, which then appears as trailing whitespace. Not sure if that is
> a big enough reason not to strip whitespace.
There's a much more important reason never _ever_ to mess with whitespace
in patches: it by definition measn that the resulting whitespace now does
not match the thing at the other end, and that _will_ mean merge problems
later (ie subsequent patches will have increasingly incorrect whitespace,
and now everybody has to live with whitespace not being reliable).
So no. The only reliable way to handle whitespace is to never corrupt it.
Don't make excuses for broken email clients etc.
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-24 8:51 ` Linus Torvalds
@ 2005-06-24 10:14 ` Tomasz Kłoczko
2005-06-24 16:40 ` Linus Torvalds
0 siblings, 1 reply; 13+ messages in thread
From: Tomasz Kłoczko @ 2005-06-24 10:14 UTC (permalink / raw)
To: Linus Torvalds
Cc: Keith Owens, Denis Vlasenko, Jeff Garzik, David Lang,
Andrew Morton, Linux Kernel, Netdev List
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2715 bytes --]
On Fri, 24 Jun 2005, Linus Torvalds wrote:
>
>
> On Fri, 24 Jun 2005, Keith Owens wrote:
>
>> On Fri, 24 Jun 2005 09:49:05 +0300,
>> Denis Vlasenko <vda@ilport.com.ua> wrote:
>>> On Friday 24 June 2005 02:33, Linus Torvalds wrote:
>>>> To actually allow real fuzz or to allow real whitespace differences in the
>>>> patch data itself is a _much_ bigger issue than this trivial patch
>>>> corruption, and I'd prefer to avoid going there if at all possible.
>>>
>>> How about automatic stripping of _trailing_ whitespace on all incoming
>>> patches? IIRC no file type (C, sh, Makefile, you name it) depends on
>>> conservation of it, thus it's 100% safe.
>>
>> One (admittedly rare) case - adding a text file that contains an
>> embedded patch, so you have a patch that includes a patch. This is
>> sometimes done in Documentation files when an external file has to be
>> changed. In embedded patch, empty lines are converted to a single
>> space, which then appears as trailing whitespace. Not sure if that is
>> a big enough reason not to strip whitespace.
>
> There's a much more important reason never _ever_ to mess with whitespace
> in patches: it by definition measn that the resulting whitespace now does
> not match the thing at the other end, and that _will_ mean merge problems
> later (ie subsequent patches will have increasingly incorrect whitespace,
> and now everybody has to live with whitespace not being reliable).
>
> So no. The only reliable way to handle whitespace is to never corrupt it.
> Don't make excuses for broken email clients etc.
Linus .. why for kernel tree can't be used indent or other source
code formater ?
If indent tool can't feet all what is neccessary or have some not ease
solveable bugs or can't be adopted other now avalaible tool IMO *it is*
*time* for start project with special formater for kernel source tree.
Using it can cut all this kind dissusions :>
If indent can handle correcly kernel source tree it will be posiible place
in source root tree one .indent.pro file and add small modificatiom to
make suit with additional "indent" target.
In case using indent neccessary patch can take only few lines :>
After this all developers before generate patches will must only pass
"make indent" .. nothing more.
As result can be also removed Documentation/CodingStyle file (description
about passing "make indent" before generate patches can be moved to
Documentation/SubmittingPatches)
kloczek
--
-----------------------------------------------------------
*Ludzie nie mają problemów, tylko sobie sami je stwarzają*
-----------------------------------------------------------
Tomasz Kłoczko, sys adm @zie.pg.gda.pl|*e-mail: kloczek@rudy.mif.pg.gda.pl*
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-24 6:49 ` Denis Vlasenko
2005-06-24 8:22 ` Keith Owens
@ 2005-06-24 15:11 ` Horst von Brand
1 sibling, 0 replies; 13+ messages in thread
From: Horst von Brand @ 2005-06-24 15:11 UTC (permalink / raw)
To: Denis Vlasenko
Cc: Linus Torvalds, Jeff Garzik, David Lang, Andrew Morton,
Linux Kernel, Netdev List
Denis Vlasenko <vda@ilport.com.ua> wrote:
> On Friday 24 June 2005 02:33, Linus Torvalds wrote:
> > To actually allow real fuzz or to allow real whitespace differences in the
> > patch data itself is a _much_ bigger issue than this trivial patch
> > corruption, and I'd prefer to avoid going there if at all possible.
>
> How about automatic stripping of _trailing_ whitespace on all incoming
> patches? IIRC no file type (C, sh, Makefile, you name it) depends on
> conservation of it, thus it's 100% safe.
Works iff the patched code is similarly mangled first... I can hear a
distant howling on LKML on the bare thought of proposing this.
You also can't assume that spaces at the end of lines make no difference
for all uses people might want to put git to.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [git patch] urgent e1000 fix
2005-06-24 10:14 ` Tomasz Kłoczko
@ 2005-06-24 16:40 ` Linus Torvalds
0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2005-06-24 16:40 UTC (permalink / raw)
To: Tomasz K³oczko
Cc: Keith Owens, Denis Vlasenko, Jeff Garzik, David Lang,
Andrew Morton, Linux Kernel, Netdev List
On Fri, 24 Jun 2005, Tomasz K³oczko wrote:
>
> Linus .. why for kernel tree can't be used indent or other source
> code formater ?
Sure, we do it, but then we try to make it obvious to all sides.
It's the "small and non-obvious" differences that are really poisonous.
You don't see them in the soruces, yet patches don't apply.
So don't do subtle whitespace "fixups" by default. It just makes everybody
unhappy down the line.
That's not to say that we don't do whitespace fixups _occasionally_. When
it's ugly enough to be noticeable, I sure as hell fix up whitespace. But
not by default.
Linus
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2005-06-24 16:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-06-23 9:24 [git patch] urgent e1000 fix Jeff Garzik
2005-06-23 21:04 ` David Lang
2005-06-23 21:19 ` Jeff Garzik
2005-06-23 22:08 ` Linus Torvalds
2005-06-23 23:10 ` Jeff Garzik
2005-06-23 23:33 ` Linus Torvalds
2005-06-24 6:49 ` Denis Vlasenko
2005-06-24 8:22 ` Keith Owens
2005-06-24 8:51 ` Linus Torvalds
2005-06-24 10:14 ` Tomasz Kłoczko
2005-06-24 16:40 ` Linus Torvalds
2005-06-24 15:11 ` Horst von Brand
2005-06-23 23:20 ` David Lang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.