list corruption on removal of snd_seq_dummy

list corruption on removal of snd_seq_dummy

[Dave Jones]

If you apply the debugging patch below (based on one from -mm)
and rmmod snd_seq_dummy, you get this spew:
(based on a 2.6.17rc6 kernel, but likely still there in todays -git)

The code in question is doing..

        __list_add(&deleted_list,
               client->ports_list_head.prev,
               client->ports_list_head.next);

which looks fishy, as those two elements aren't going to be consecutive,
as __list_add expects.

		Dave

List corruption. next->prev should be f76896e8, but was f70acbcc
------------[ cut here ]------------
kernel BUG at include/linux/list.h:58!
invalid opcode: 0000 [#1]
SMP
last sysfs file: /devices/pci0000:00/0000:00:1f.3/i2c-0/0-002e/pwm3
Modules linked in: lm85 hwmon_vid hwmon i2c_isa ipv6 nls_utf8 loop snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss matroxfb_base matroxfb_DAC1064 snd_mixer_oss matroxfb_accel matroxfb_Ti3026 matroxfb_g450 g450_pll matroxfb_misc snd_pcm 3w_9xxx e1000 snd_timer snd i2c_i801 soundcore i2c_core snd_page_alloc ext3 jbd 3w_xxxx ata_piix libata sd_mod scsi_mod
CPU:    1
EIP:    0060:[<f8a1a56d>]    Not tainted VLI
EFLAGS: 00010092   (2.6.16-1.2232_FC6 #1)
EIP is at snd_seq_delete_all_ports+0x57/0x167 [snd_seq]
eax: 00000044   ebx: f76896e8   ecx: c06c77d0   edx: 00000096
esi: f76896e8   edi: f70acbcc   ebp: f70acb50   esp: f772cf08
ds: 007b   es: 007b   ss: 0068
Process rmmod (pid: 6274, threadinfo=f772c000 task=df26f550)
Stack: f8a1b83b f76896e8 f70acbcc f70acbe4 f70acbd4 00000282 22222222 22222222
       f70acb50 00000020 00000000 f772c000 f8a1521e f70acb50 f8a152e2 c043df66
       f772c000 c0445bf0 f70acb50 f8a1778b f89b4e00 c043e177 5f646e73 5f716573
Call Trace:
 <f8a1521e> seq_free_client1+0x8/0x90 [snd_seq]  <f8a152e2> seq_free_client+0x3c/0x78 [snd_seq]
 <c043df66> __try_stop_module+0x0/0x44  <c0445bf0> stop_machine_run+0x2e/0x34
 <f8a1778b> snd_seq_delete_kernel_client+0x1a/0x2c [snd_seq]  <c043e177> sys_delete_module+0x192/0x1bb
 <c060f020> do_page_fault+0x235/0x5ba  <c045ab3f> do_munmap+0x196/0x1af
 <c0403e1f> syscall_call+0x7/0xb
Code: 24 14 39 7d 7c 74 67 8b 5d 7c 8b b5 80 00 00 00 8b 43 04 39 f0 74 1c 89 74 24 04 89 44 24 08 c7 04 24 3b b8 a1 f8 e8 e7 ab a0 c7 <0f> 0b 3a 00 26 b8 a1 f8 8b 06 39 d8 74 1c 89 5c 24 04 89 44 24
EIP: [<f8a1a56d>] snd_seq_delete_all_ports+0x57/0x167 [snd_seq] SS:ESP 0068:f772cf08
 <3>BUG: sleeping function called from invalid context at include/linux/rwsem.h:43
in_atomic():0, irqs_disabled():1
 <c042ffa2> blocking_notifier_call_chain+0x18/0x4b  <c0427387> do_exit+0x19/0x7a5
 <c053a76b> do_unblank_screen+0x2a/0x127  <c040549a> die+0x2a5/0x2ca
 <c0405b30> do_invalid_op+0x0/0xab  <c0405bd2> do_invalid_op+0xa2/0xab
 <f8a1a56d> snd_seq_delete_all_ports+0x57/0x167 [snd_seq]  <c041e25f> find_busiest_group+0xfc/0x29c
 <c060dfd0> _spin_unlock_irq+0x5/0x7  <c04390ef> debug_mutex_add_waiter+0x97/0xa9
 <f8a1a531> snd_seq_delete_all_ports+0x1b/0x167 [snd_seq]  <c04049b7> error_code+0x4f/0x54
 <f8a1a56d> snd_seq_delete_all_ports+0x57/0x167 [snd_seq]  <f8a1521e> seq_free_client1+0x8/0x90 [snd_seq]
 <f8a152e2> seq_free_client+0x3c/0x78 [snd_seq]  <c043df66> __try_stop_module+0x0/0x44
 <c0445bf0> stop_machine_run+0x2e/0x34  <f8a1778b> snd_seq_delete_kernel_client+0x1a/0x2c [snd_seq]
 <c043e177> sys_delete_module+0x192/0x1bb  <c060f020> do_page_fault+0x235/0x5ba
 <c045ab3f> do_munmap+0x196/0x1af  <c0403e1f> syscall_call+0x7/0xb
BUG: rmmod/6274, lock held at task exit time!
 [f8a21fa0] {register_mutex}
.. held by:             rmmod: 6274 [df26f550, 117]
... acquired at:               seq_free_client+0x10/0x78 [snd_seq]



--- linux-2.6.12/include/linux/list.h~	2005-08-08 15:34:50.000000000 -0400
+++ linux-2.6.12/include/linux/list.h	2005-08-08 15:35:22.000000000 -0400
@@ -5,7 +5,9 @@
 
 #include <linux/stddef.h>
 #include <linux/prefetch.h>
+#include <linux/kernel.h>
 #include <asm/system.h>
+#include <asm/bug.h>
 
 /*
  * These are non-NULL pointers that will result in page faults
@@ -52,6 +52,16 @@ static inline void __list_add(struct lis
 			      struct list_head *prev,
 			      struct list_head *next)
 {
+	if (next->prev != prev) {
+		printk("List corruption. next->prev should be %p, but was %p\n",
+				prev, next->prev);
+		BUG();
+	}
+	if (prev->next != next) {
+		printk("List corruption. prev->next should be %p, but was %p\n",
+				next, prev->next);
+		BUG();
+	}
 	next->prev = new;
 	new->next = next;
 	new->prev = prev;
@@ -162,6 +162,16 @@ static inline void __list_del(struct lis
  */
 static inline void list_del(struct list_head *entry)
 {
+	if (entry->prev->next != entry) {
+		printk("List corruption. prev->next should be %p, but was %p\n",
+				entry, entry->prev->next);
+		BUG();
+	}
+	if (entry->next->prev != entry) {
+		printk("List corruption. next->prev should be %p, but was %p\n",
+				entry, entry->next->prev);
+		BUG();
+	}
 	__list_del(entry->prev, entry->next);
 	entry->next = LIST_POISON1;
 	entry->prev = LIST_POISON2;


-- 
http://www.codemonkey.org.uk

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Thu, 22 Jun 2006 23:15:26 -0400,
Dave Jones wrote:
> 
> If you apply the debugging patch below (based on one from -mm)
> and rmmod snd_seq_dummy, you get this spew:
> (based on a 2.6.17rc6 kernel, but likely still there in todays -git)
> 
> The code in question is doing..
> 
>         __list_add(&deleted_list,
>                client->ports_list_head.prev,
>                client->ports_list_head.next);
> 
> which looks fishy, as those two elements aren't going to be consecutive,
> as __list_add expects.

I think the code behaves correctly but probably misusing __list_add().
It movies the whole entries from an existing list_head A
(clients->ports_list_head) to a new list_head B (deleted_list).
The above is exapnded:

	A->next->prev = B;
	B->next = A->next;
	B->prev = A->prev;
	A->prev->next = B;


Any better way to achieve it using standard macros?


thanks,

Takashi

> 
> 		Dave
> 
> List corruption. next->prev should be f76896e8, but was f70acbcc
> ------------[ cut here ]------------
> kernel BUG at include/linux/list.h:58!
> invalid opcode: 0000 [#1]
> SMP
> last sysfs file: /devices/pci0000:00/0000:00:1f.3/i2c-0/0-002e/pwm3
> Modules linked in: lm85 hwmon_vid hwmon i2c_isa ipv6 nls_utf8 loop snd_seq_dummy snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss matroxfb_base matroxfb_DAC1064 snd_mixer_oss matroxfb_accel matroxfb_Ti3026 matroxfb_g450 g450_pll matroxfb_misc snd_pcm 3w_9xxx e1000 snd_timer snd i2c_i801 soundcore i2c_core snd_page_alloc ext3 jbd 3w_xxxx ata_piix libata sd_mod scsi_mod
> CPU:    1
> EIP:    0060:[<f8a1a56d>]    Not tainted VLI
> EFLAGS: 00010092   (2.6.16-1.2232_FC6 #1)
> EIP is at snd_seq_delete_all_ports+0x57/0x167 [snd_seq]
> eax: 00000044   ebx: f76896e8   ecx: c06c77d0   edx: 00000096
> esi: f76896e8   edi: f70acbcc   ebp: f70acb50   esp: f772cf08
> ds: 007b   es: 007b   ss: 0068
> Process rmmod (pid: 6274, threadinfo=f772c000 task=df26f550)
> Stack: f8a1b83b f76896e8 f70acbcc f70acbe4 f70acbd4 00000282 22222222 22222222
>        f70acb50 00000020 00000000 f772c000 f8a1521e f70acb50 f8a152e2 c043df66
>        f772c000 c0445bf0 f70acb50 f8a1778b f89b4e00 c043e177 5f646e73 5f716573
> Call Trace:
>  <f8a1521e> seq_free_client1+0x8/0x90 [snd_seq]  <f8a152e2> seq_free_client+0x3c/0x78 [snd_seq]
>  <c043df66> __try_stop_module+0x0/0x44  <c0445bf0> stop_machine_run+0x2e/0x34
>  <f8a1778b> snd_seq_delete_kernel_client+0x1a/0x2c [snd_seq]  <c043e177> sys_delete_module+0x192/0x1bb
>  <c060f020> do_page_fault+0x235/0x5ba  <c045ab3f> do_munmap+0x196/0x1af
>  <c0403e1f> syscall_call+0x7/0xb
> Code: 24 14 39 7d 7c 74 67 8b 5d 7c 8b b5 80 00 00 00 8b 43 04 39 f0 74 1c 89 74 24 04 89 44 24 08 c7 04 24 3b b8 a1 f8 e8 e7 ab a0 c7 <0f> 0b 3a 00 26 b8 a1 f8 8b 06 39 d8 74 1c 89 5c 24 04 89 44 24
> EIP: [<f8a1a56d>] snd_seq_delete_all_ports+0x57/0x167 [snd_seq] SS:ESP 0068:f772cf08
>  <3>BUG: sleeping function called from invalid context at include/linux/rwsem.h:43
> in_atomic():0, irqs_disabled():1
>  <c042ffa2> blocking_notifier_call_chain+0x18/0x4b  <c0427387> do_exit+0x19/0x7a5
>  <c053a76b> do_unblank_screen+0x2a/0x127  <c040549a> die+0x2a5/0x2ca
>  <c0405b30> do_invalid_op+0x0/0xab  <c0405bd2> do_invalid_op+0xa2/0xab
>  <f8a1a56d> snd_seq_delete_all_ports+0x57/0x167 [snd_seq]  <c041e25f> find_busiest_group+0xfc/0x29c
>  <c060dfd0> _spin_unlock_irq+0x5/0x7  <c04390ef> debug_mutex_add_waiter+0x97/0xa9
>  <f8a1a531> snd_seq_delete_all_ports+0x1b/0x167 [snd_seq]  <c04049b7> error_code+0x4f/0x54
>  <f8a1a56d> snd_seq_delete_all_ports+0x57/0x167 [snd_seq]  <f8a1521e> seq_free_client1+0x8/0x90 [snd_seq]
>  <f8a152e2> seq_free_client+0x3c/0x78 [snd_seq]  <c043df66> __try_stop_module+0x0/0x44
>  <c0445bf0> stop_machine_run+0x2e/0x34  <f8a1778b> snd_seq_delete_kernel_client+0x1a/0x2c [snd_seq]
>  <c043e177> sys_delete_module+0x192/0x1bb  <c060f020> do_page_fault+0x235/0x5ba
>  <c045ab3f> do_munmap+0x196/0x1af  <c0403e1f> syscall_call+0x7/0xb
> BUG: rmmod/6274, lock held at task exit time!
>  [f8a21fa0] {register_mutex}
> .. held by:             rmmod: 6274 [df26f550, 117]
> ... acquired at:               seq_free_client+0x10/0x78 [snd_seq]
> 
> 
> 
> --- linux-2.6.12/include/linux/list.h~	2005-08-08 15:34:50.000000000 -0400
> +++ linux-2.6.12/include/linux/list.h	2005-08-08 15:35:22.000000000 -0400
> @@ -5,7 +5,9 @@
>  
>  #include <linux/stddef.h>
>  #include <linux/prefetch.h>
> +#include <linux/kernel.h>
>  #include <asm/system.h>
> +#include <asm/bug.h>
>  
>  /*
>   * These are non-NULL pointers that will result in page faults
> @@ -52,6 +52,16 @@ static inline void __list_add(struct lis
>  			      struct list_head *prev,
>  			      struct list_head *next)
>  {
> +	if (next->prev != prev) {
> +		printk("List corruption. next->prev should be %p, but was %p\n",
> +				prev, next->prev);
> +		BUG();
> +	}
> +	if (prev->next != next) {
> +		printk("List corruption. prev->next should be %p, but was %p\n",
> +				next, prev->next);
> +		BUG();
> +	}
>  	next->prev = new;
>  	new->next = next;
>  	new->prev = prev;
> @@ -162,6 +162,16 @@ static inline void __list_del(struct lis
>   */
>  static inline void list_del(struct list_head *entry)
>  {
> +	if (entry->prev->next != entry) {
> +		printk("List corruption. prev->next should be %p, but was %p\n",
> +				entry, entry->prev->next);
> +		BUG();
> +	}
> +	if (entry->next->prev != entry) {
> +		printk("List corruption. next->prev should be %p, but was %p\n",
> +				entry, entry->next->prev);
> +		BUG();
> +	}
>  	__list_del(entry->prev, entry->next);
>  	entry->next = LIST_POISON1;
>  	entry->prev = LIST_POISON2;
> 
> 
> -- 
> http://www.codemonkey.org.uk
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: list corruption on removal of snd_seq_dummy

[Dave Jones]

On Fri, Jun 23, 2006 at 12:47:46PM +0200, Takashi Iwai wrote:
 
 > > The code in question is doing..
 > > 
 > >         __list_add(&deleted_list,
 > >                client->ports_list_head.prev,
 > >                client->ports_list_head.next);
 > > 
 > > which looks fishy, as those two elements aren't going to be consecutive,
 > > as __list_add expects.
 > 
 > I think the code behaves correctly but probably misusing __list_add().
 > It movies the whole entries from an existing list_head A
 > (clients->ports_list_head) to a new list_head B (deleted_list).
 > The above is exapnded:
 > 
 > 	A->next->prev = B;
 > 	B->next = A->next;
 > 	B->prev = A->prev;
 > 	A->prev->next = B;
 > 
 > Any better way to achieve it using standard macros?

Why can't you just list_move() the elements ?

		Dave

-- 
http://www.codemonkey.org.uk

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

Re: list corruption on removal of snd_seq_dummy

[Dave Jones]

On Fri, Jun 23, 2006 at 12:47:46PM +0200, Takashi Iwai wrote:
 
 > > The code in question is doing..
 > > 
 > >         __list_add(&deleted_list,
 > >                client->ports_list_head.prev,
 > >                client->ports_list_head.next);
 > > 
 > > which looks fishy, as those two elements aren't going to be consecutive,
 > > as __list_add expects.
 > 
 > I think the code behaves correctly but probably misusing __list_add().
 > It movies the whole entries from an existing list_head A
 > (clients->ports_list_head) to a new list_head B (deleted_list).
 > The above is exapnded:
 > 
 > 	A->next->prev = B;
 > 	B->next = A->next;
 > 	B->prev = A->prev;
 > 	A->prev->next = B;
 > 
 > Any better way to achieve it using standard macros?

Why can't you just list_move() the elements ?

		Dave

-- 
http://www.codemonkey.org.uk

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Mon, 26 Jun 2006 23:27:38 -0400,
Dave Jones wrote:
> 
> On Fri, Jun 23, 2006 at 12:47:46PM +0200, Takashi Iwai wrote:
>  
>  > > The code in question is doing..
>  > > 
>  > >         __list_add(&deleted_list,
>  > >                client->ports_list_head.prev,
>  > >                client->ports_list_head.next);
>  > > 
>  > > which looks fishy, as those two elements aren't going to be consecutive,
>  > > as __list_add expects.
>  > 
>  > I think the code behaves correctly but probably misusing __list_add().
>  > It movies the whole entries from an existing list_head A
>  > (clients->ports_list_head) to a new list_head B (deleted_list).
>  > The above is exapnded:
>  > 
>  > 	A->next->prev = B;
>  > 	B->next = A->next;
>  > 	B->prev = A->prev;
>  > 	A->prev->next = B;
>  > 
>  > Any better way to achieve it using standard macros?
> 
> Why can't you just list_move() the elements ?

No, list_move() can't move the whole elements without loop.

A solution is

	list_add(B, A);
	list_del_init(A);

(although this introduces a bit more code :)


Takashi

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Mon, 26 Jun 2006 23:27:38 -0400,
Dave Jones wrote:
> 
> On Fri, Jun 23, 2006 at 12:47:46PM +0200, Takashi Iwai wrote:
>  
>  > > The code in question is doing..
>  > > 
>  > >         __list_add(&deleted_list,
>  > >                client->ports_list_head.prev,
>  > >                client->ports_list_head.next);
>  > > 
>  > > which looks fishy, as those two elements aren't going to be consecutive,
>  > > as __list_add expects.
>  > 
>  > I think the code behaves correctly but probably misusing __list_add().
>  > It movies the whole entries from an existing list_head A
>  > (clients->ports_list_head) to a new list_head B (deleted_list).
>  > The above is exapnded:
>  > 
>  > 	A->next->prev = B;
>  > 	B->next = A->next;
>  > 	B->prev = A->prev;
>  > 	A->prev->next = B;
>  > 
>  > Any better way to achieve it using standard macros?
> 
> Why can't you just list_move() the elements ?

No, list_move() can't move the whole elements without loop.

A solution is

	list_add(B, A);
	list_del_init(A);

(although this introduces a bit more code :)


Takashi

Re: list corruption on removal of snd_seq_dummy

[Chuck Ebbert]

In-Reply-To: <s5hejxb7xrb.wl%tiwai@suse.de>

On Tue, 27 Jun 2006 11:28:56 +0200, Takashi Iwai wrote:

> >  > > The code in question is doing..
> >  > > 
> >  > >         __list_add(&deleted_list,
> >  > >                client->ports_list_head.prev,
> >  > >                client->ports_list_head.next);
> >  > > 
> >  > > which looks fishy, as those two elements aren't going to be consecutive,
> >  > > as __list_add expects.
> >  > 
> >  > I think the code behaves correctly but probably misusing __list_add().
> >  > It movies the whole entries from an existing list_head A
> >  > (clients->ports_list_head) to a new list_head B (deleted_list).
> >  > The above is exapnded:
> >  > 
> >  >  A->next->prev = B;
> >  >  B->next = A->next;
> >  >  B->prev = A->prev;
> >  >  A->prev->next = B;
> >  > 
> >  > Any better way to achieve it using standard macros?
> > 
> > Why can't you just list_move() the elements ?
> 
> No, list_move() can't move the whole elements without loop.
> 
> A solution is
> 
>       list_add(B, A);
>       list_del_init(A);
> 
> (although this introduces a bit more code :)

Shouldn't it be like this?

        ports_list_first = client->ports_list_head.next;
        list_del_init(client->ports_list_head);
        list_splice(ports_list_first, &deleted_list);

-- 
Chuck
 "You can't read a newspaper if you can't read."  --George W. Bush

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Tue, 27 Jun 2006 08:05:01 -0400,
Chuck Ebbert wrote:
> 
> In-Reply-To: <s5hejxb7xrb.wl%tiwai@suse.de>
> 
> On Tue, 27 Jun 2006 11:28:56 +0200, Takashi Iwai wrote:
> 
> > >  > > The code in question is doing..
> > >  > > 
> > >  > >         __list_add(&deleted_list,
> > >  > >                client->ports_list_head.prev,
> > >  > >                client->ports_list_head.next);
> > >  > > 
> > >  > > which looks fishy, as those two elements aren't going to be consecutive,
> > >  > > as __list_add expects.
> > >  > 
> > >  > I think the code behaves correctly but probably misusing __list_add().
> > >  > It movies the whole entries from an existing list_head A
> > >  > (clients->ports_list_head) to a new list_head B (deleted_list).
> > >  > The above is exapnded:
> > >  > 
> > >  >  A->next->prev = B;
> > >  >  B->next = A->next;
> > >  >  B->prev = A->prev;
> > >  >  A->prev->next = B;
> > >  > 
> > >  > Any better way to achieve it using standard macros?
> > > 
> > > Why can't you just list_move() the elements ?
> > 
> > No, list_move() can't move the whole elements without loop.
> > 
> > A solution is
> > 
> >       list_add(B, A);
> >       list_del_init(A);
> > 
> > (although this introduces a bit more code :)
> 
> Shouldn't it be like this?
> 
>         ports_list_first = client->ports_list_head.next;
>         list_del_init(client->ports_list_head);
>         list_splice(ports_list_first, &deleted_list);

This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
a longer code :)


Takashi

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Tue, 27 Jun 2006 08:05:01 -0400,
Chuck Ebbert wrote:
> 
> In-Reply-To: <s5hejxb7xrb.wl%tiwai@suse.de>
> 
> On Tue, 27 Jun 2006 11:28:56 +0200, Takashi Iwai wrote:
> 
> > >  > > The code in question is doing..
> > >  > > 
> > >  > >         __list_add(&deleted_list,
> > >  > >                client->ports_list_head.prev,
> > >  > >                client->ports_list_head.next);
> > >  > > 
> > >  > > which looks fishy, as those two elements aren't going to be consecutive,
> > >  > > as __list_add expects.
> > >  > 
> > >  > I think the code behaves correctly but probably misusing __list_add().
> > >  > It movies the whole entries from an existing list_head A
> > >  > (clients->ports_list_head) to a new list_head B (deleted_list).
> > >  > The above is exapnded:
> > >  > 
> > >  >  A->next->prev = B;
> > >  >  B->next = A->next;
> > >  >  B->prev = A->prev;
> > >  >  A->prev->next = B;
> > >  > 
> > >  > Any better way to achieve it using standard macros?
> > > 
> > > Why can't you just list_move() the elements ?
> > 
> > No, list_move() can't move the whole elements without loop.
> > 
> > A solution is
> > 
> >       list_add(B, A);
> >       list_del_init(A);
> > 
> > (although this introduces a bit more code :)
> 
> Shouldn't it be like this?
> 
>         ports_list_first = client->ports_list_head.next;
>         list_del_init(client->ports_list_head);
>         list_splice(ports_list_first, &deleted_list);

This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
a longer code :)


Takashi

Re: list corruption on removal of snd_seq_dummy

[Dave Jones]

On Tue, Jun 27, 2006 at 02:38:39PM +0200, Takashi Iwai wrote:
 > > > No, list_move() can't move the whole elements without loop.
 > > > 
 > > > A solution is
 > > > 
 > > >       list_add(B, A);
 > > >       list_del_init(A);
 > > > 
 > > > (although this introduces a bit more code :)
 > > 
 > > Shouldn't it be like this?
 > > 
 > >         ports_list_first = client->ports_list_head.next;
 > >         list_del_init(client->ports_list_head);
 > >         list_splice(ports_list_first, &deleted_list);
 > 
 > This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
 > a longer code :)

This is hardly a speed/size critical function. I'd go for readability
over cute hacks any day.

		Dave

-- 
http://www.codemonkey.org.uk

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

Re: list corruption on removal of snd_seq_dummy

[Dave Jones]

On Tue, Jun 27, 2006 at 02:38:39PM +0200, Takashi Iwai wrote:
 > > > No, list_move() can't move the whole elements without loop.
 > > > 
 > > > A solution is
 > > > 
 > > >       list_add(B, A);
 > > >       list_del_init(A);
 > > > 
 > > > (although this introduces a bit more code :)
 > > 
 > > Shouldn't it be like this?
 > > 
 > >         ports_list_first = client->ports_list_head.next;
 > >         list_del_init(client->ports_list_head);
 > >         list_splice(ports_list_first, &deleted_list);
 > 
 > This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
 > a longer code :)

This is hardly a speed/size critical function. I'd go for readability
over cute hacks any day.

		Dave

-- 
http://www.codemonkey.org.uk

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Tue, 27 Jun 2006 12:58:19 -0400,
Dave Jones wrote:
> 
> On Tue, Jun 27, 2006 at 02:38:39PM +0200, Takashi Iwai wrote:
>  > > > No, list_move() can't move the whole elements without loop.
>  > > > 
>  > > > A solution is
>  > > > 
>  > > >       list_add(B, A);
>  > > >       list_del_init(A);
>  > > > 
>  > > > (although this introduces a bit more code :)
>  > > 
>  > > Shouldn't it be like this?
>  > > 
>  > >         ports_list_first = client->ports_list_head.next;
>  > >         list_del_init(client->ports_list_head);
>  > >         list_splice(ports_list_first, &deleted_list);
>  > 
>  > This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
>  > a longer code :)
> 
> This is hardly a speed/size critical function. I'd go for readability
> over cute hacks any day.

Yeah, of course.
The code was already fixed on ALSA tree to use standard macros.


Takashi

Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

Re: list corruption on removal of snd_seq_dummy

[Takashi Iwai]

At Tue, 27 Jun 2006 12:58:19 -0400,
Dave Jones wrote:
> 
> On Tue, Jun 27, 2006 at 02:38:39PM +0200, Takashi Iwai wrote:
>  > > > No, list_move() can't move the whole elements without loop.
>  > > > 
>  > > > A solution is
>  > > > 
>  > > >       list_add(B, A);
>  > > >       list_del_init(A);
>  > > > 
>  > > > (although this introduces a bit more code :)
>  > > 
>  > > Shouldn't it be like this?
>  > > 
>  > >         ports_list_first = client->ports_list_head.next;
>  > >         list_del_init(client->ports_list_head);
>  > >         list_splice(ports_list_first, &deleted_list);
>  > 
>  > This requires INIT_LIST_HEAD(&deleted_list) first, so obviously
>  > a longer code :)
> 
> This is hardly a speed/size critical function. I'd go for readability
> over cute hacks any day.

Yeah, of course.
The code was already fixed on ALSA tree to use standard macros.


Takashi