The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assox_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
---
net/ieee80211/softmac/ieee80211softmac_assoc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
From: Michael Buesch <hidden> Date: 2006-12-10 17:50:15
On Sunday 10 December 2006 18:39, Ulrich Kunitz wrote:
quoted hunk
The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assox_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
---
net/ieee80211/softmac/ieee80211softmac_assoc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
On Sunday 10 December 2006 18:39, Ulrich Kunitz wrote:
quoted
The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assox_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
---
net/ieee80211/softmac/ieee80211softmac_assoc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
Why do you use a zero delay here? What does that fix?
The problem is that you there are now different work structures:
struct work_struct and struct delayed_work. The quick fix seems to
have been to change all old work_structs as associnfo's work to
delayed_work. The way the structures are designed calling
schedule_work or schedule_delayed_work doesn't matter, but you
will get a gcc warning, because the pointer types are not
identical. This change works around the warning in the same way as
the other schedule_work calls for associnfo's work.
I'm not sure, whether the breaking of the workqueue API is really
worth it. What I see is that the change introduced choices and
choices make things more complex.
--
Uli Kunitz
From: Michael Buesch <hidden> Date: 2006-12-10 18:41:13
On Sunday 10 December 2006 19:35, Ulrich Kunitz wrote:
On 06-12-10 18:49 Michael Buesch wrote:
quoted
On Sunday 10 December 2006 18:39, Ulrich Kunitz wrote:
quoted
The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assox_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
---
net/ieee80211/softmac/ieee80211softmac_assoc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
Why do you use a zero delay here? What does that fix?
The problem is that you there are now different work structures:
struct work_struct and struct delayed_work. The quick fix seems to
have been to change all old work_structs as associnfo's work to
delayed_work. The way the structures are designed calling
schedule_work or schedule_delayed_work doesn't matter, but you
will get a gcc warning, because the pointer types are not
identical. This change works around the warning in the same way as
the other schedule_work calls for associnfo's work.
I'm not sure, whether the breaking of the workqueue API is really
worth it. What I see is that the change introduced choices and
choices make things more complex.
Ah, ok. Thanks for the clarification. In this case this patch is
ACKed as well.
--
Greetings Michael.
From: Andrew Morton <hidden> Date: 2006-12-10 18:42:31
On Sun, 10 Dec 2006 19:35:36 +0100
Ulrich Kunitz [off-list ref] wrote:
The problem is that you there are now different work structures:
struct work_struct and struct delayed_work. The quick fix seems to
have been to change all old work_structs as associnfo's work to
delayed_work. The way the structures are designed calling
schedule_work or schedule_delayed_work doesn't matter, but you
will get a gcc warning, because the pointer types are not
identical. This change works around the warning in the same way as
the other schedule_work calls for associnfo's work.
David proposed the below. Does it fix things for you?
I'm not sure, whether the breaking of the workqueue API is really
worth it. What I see is that the change introduced choices and
choices make things more complex.
It is kinda sucky. But it saves a bit of space in kernel data structures.
From: Larry Finger <hidden> Date: 2006-12-11 04:24:39
Ulrich Kunitz wrote:
The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assox_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
Thanks Ulrich for this patch. I had spent the better part of 2 days bisecting Linus's git tree
trying to isolate the problem that kept the system from booting when my bcm43xx card was installed.
I thought that when someone _BROKE_ an interface with this kind of change, it was their duty to fix
_ALL_ parts of the system that uses this facility. At a minimum, shouldn't all maintainers get a
heads up? I don't subscribe to LKML, but I peruse the summary and I certainly do not recall seeing a
warning that this change was coming.
Larry
From: Larry Finger <hidden> Date: 2006-12-11 17:34:32
Andrew Morton wrote:
quoted hunk
On Sun, 10 Dec 2006 19:35:36 +0100
Ulrich Kunitz [off-list ref] wrote:
quoted
The problem is that you there are now different work structures:
struct work_struct and struct delayed_work. The quick fix seems to
have been to change all old work_structs as associnfo's work to
delayed_work. The way the structures are designed calling
schedule_work or schedule_delayed_work doesn't matter, but you
will get a gcc warning, because the pointer types are not
identical. This change works around the warning in the same way as
the other schedule_work calls for associnfo's work.
David proposed the below. Does it fix things for you?
I'm not sure, whether the breaking of the workqueue API is really
worth it. What I see is that the change introduced choices and
choices make things more complex.
It is kinda sucky. But it saves a bit of space in kernel data structures.
The above patch fixes a compile problem; however, the code still hangs when the network is started.
You need two additional hunks as shown below.
Larry
=========================================================================
From: Ulrich Kunitz <redacted>
The signature of work functions changed recently from a context
pointer to the work structure pointer. This caused a problem in
the ieee80211softmac code, because the ieee80211softmac_assoc_work
function has been called directly with a parameter explicitly
casted to (void*). This compiled correctly but resulted in a
softlock, because mutex_lock was called with the wrong memory
address. The patch fixes the problem. Another issue was a wrong
call of the schedule_work function. Softmac works again and this
fixes the problem I mentioned earlier in the zd1211rw rx tasklet
patch. The patch is against Linus' tree (commit af1713e0).
Signed-off-by: Ulrich Kunitz <redacted>
Acked-by: Michael Buesch <redacted>
Signed-off-by: Larry Finger <redacted>
---
From: John W. Linville <hidden> Date: 2006-12-11 22:12:25
On Sun, Dec 10, 2006 at 10:24:27PM -0600, Larry Finger wrote:
I thought that when someone _BROKE_ an interface with this kind of change,
it was their duty to fix _ALL_ parts of the system that uses this facility.
At a minimum, shouldn't all maintainers get a heads up? I don't subscribe
to LKML, but I peruse the summary and I certainly do not recall seeing a
warning that this change was coming.
FWIW, there was some coverage on LWN.
--
John W. Linville
linville@tuxdriver.com