* Re: [PATCH net-next 1/2] ipv6: remove unnecessary codes in tcp_ipv6.c
From: David Miller @ 2012-07-02 3:26 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <1341199140-17135-1-git-send-email-roy.qing.li@gmail.com>
From: roy.qing.li@gmail.com
Date: Mon, 2 Jul 2012 11:18:59 +0800
> - if (opt) {
> - newnp->opt = ipv6_dup_options(newsk, opt);
> - if (opt != np->opt)
> - sock_kfree_s(sk, opt, opt->tot_len);
This is bogus, if we copy the options into a new copy in
ipv6_dup_options() we have to free the old one or else we
leak it.
^ permalink raw reply
* Re: [PATCH net-next 2/2] dccp: remove unnecessary codes in ipv6.c
From: David Miller @ 2012-07-02 3:26 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <1341199140-17135-2-git-send-email-roy.qing.li@gmail.com>
From: roy.qing.li@gmail.com
Date: Mon, 2 Jul 2012 11:19:00 +0800
> - if (opt != NULL) {
> - newnp->opt = ipv6_dup_options(newsk, opt);
> - if (opt != np->opt)
> - sock_kfree_s(sk, opt, opt->tot_len);
> - }
Same bug as your other patch, this leaks options.
^ permalink raw reply
* Re: linux-next BUG: held lock freed!
From: Fengguang Wu @ 2012-07-02 3:39 UTC (permalink / raw)
To: Christoph Lameter
Cc: Trond Myklebust, J. Bruce Fields, linux-nfs, LKML, netdev,
Pekka Enberg, Linux Memory Management List, Stephen Rothwell
In-Reply-To: <20120702025625.GA6531@localhost>
On Mon, Jul 02, 2012 at 10:56:25AM +0800, Fengguang Wu wrote:
> Hi all,
>
> More observations on this bug:
>
> The slab tree itself actually boots fine. So Christoph's commit may be
> merely disclosing some bug hidden in another for-next tree which
> happens to be merged before the slab tree..
Sorry: the bug does appear in the standalone slab tree, where the
dmesg is
[ 307.648802] blkid (2963) used greatest stack depth: 2832 bytes left
[ 307.892070] vhci_hcd: changed 0
[ 308.766647]
[ 308.766648] =========================
[ 308.766649] [ BUG: held lock freed! ]
[ 308.766651] 3.5.0-rc1+ #44 Not tainted
[ 308.766651] -------------------------
[ 308.766653] mtd_probe/3040 is freeing memory ffff880006defdd0-ffff880006df0dcf, with a lock still held there!
[ 308.766662] (&type->s_umount_key#31/1){+.+.+.}, at: [<ffffffff81187166>] sget+0x299/0x463
[ 308.766663] 3 locks held by mtd_probe/3040:
[ 308.766667] #0: (&type->s_umount_key#31/1){+.+.+.}, at: [<ffffffff81187166>] sget+0x299/0x463
[ 308.766671] #1: (sb_lock){+.+.-.}, at: [<ffffffff81186f00>] sget+0x33/0x463
[ 308.766675] #2: (unnamed_dev_lock){+.+...}, at: [<ffffffff81186711>] get_anon_bdev+0x38/0xe8
[ 308.766675]
[ 308.766675] stack backtrace:
[ 308.766677] Pid: 3040, comm: mtd_probe Not tainted 3.5.0-rc1+ #44
[ 308.766678] Call Trace:
[ 308.766683] [<ffffffff810ddc6e>] debug_check_no_locks_freed+0x109/0x14b
[ 308.766703] [<ffffffff81173f7c>] kmem_cache_free+0x2e/0xa7
[ 308.766708] [<ffffffff816a5d9d>] ida_get_new_above+0x173/0x184
[ 308.766711] [<ffffffff810db9a4>] ? lock_acquired+0x1e4/0x219
[ 308.766713] [<ffffffff81186727>] get_anon_bdev+0x4e/0xe8
[ 308.766715] [<ffffffff811867d8>] set_anon_super+0x17/0x2a
[ 308.766717] [<ffffffff81187270>] sget+0x3a3/0x463
[ 308.766719] [<ffffffff811867c1>] ? get_anon_bdev+0xe8/0xe8
[ 308.766722] [<ffffffff811a1fbe>] mount_pseudo+0x31/0x152
[ 308.766727] [<ffffffff81cb1f54>] mtd_inodefs_mount+0x24/0x26
[ 308.766729] [<ffffffff81187e34>] mount_fs+0x69/0x155
[ 308.766733] [<ffffffff811531b2>] ? __alloc_percpu+0x10/0x12
[ 308.766736] [<ffffffff8119ca4c>] vfs_kern_mount+0x62/0xd9
[ 308.766739] [<ffffffff811a1b43>] simple_pin_fs+0x4c/0x9b
[ 308.766741] [<ffffffff81cb338a>] mtdchar_open+0x42/0x188
[ 308.766744] [<ffffffff811886ef>] chrdev_open+0x11f/0x14a
[ 308.766747] [<ffffffff810c0880>] ? local_clock+0x19/0x52
[ 308.766750] [<ffffffff811885d0>] ? cdev_put+0x26/0x26
[ 308.766752] [<ffffffff811836cc>] do_dentry_open+0x1e4/0x2b2
[ 308.766754] [<ffffffff8118434a>] nameidata_to_filp+0x5e/0xa3
[ 308.766756] [<ffffffff8119118f>] do_last+0x68f/0x6d3
[ 308.766759] [<ffffffff811912d8>] path_openat+0xd2/0x32a
[ 308.766762] [<ffffffff8111eed8>] ? time_hardirqs_off+0x26/0x2a
[ 308.766765] [<ffffffff810d9e88>] ? trace_hardirqs_off+0xd/0xf
[ 308.766767] [<ffffffff81191630>] do_filp_open+0x38/0x86
[ 308.766771] [<ffffffff82e95e22>] ? _raw_spin_unlock+0x28/0x3b
[ 308.766773] [<ffffffff8119baa7>] ? alloc_fd+0xe5/0xf7
[ 308.766776] [<ffffffff811843fd>] do_sys_open+0x6e/0xfb
[ 308.766777] [<ffffffff811844ab>] sys_open+0x21/0x23
[ 308.766780] [<ffffffff82e9cb69>] system_call_fastpath+0x16/0x1b
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: linux-next BUG: held lock freed!
From: Fengguang Wu @ 2012-07-02 3:42 UTC (permalink / raw)
To: Christoph Lameter
Cc: Trond Myklebust, J. Bruce Fields, linux-nfs, LKML, netdev,
Pekka Enberg, Linux Memory Management List, Stephen Rothwell
In-Reply-To: <20120702033914.GA7433@localhost>
On Mon, Jul 02, 2012 at 11:39:14AM +0800, Fengguang Wu wrote:
> On Mon, Jul 02, 2012 at 10:56:25AM +0800, Fengguang Wu wrote:
> > Hi all,
> >
> > More observations on this bug:
> >
> > The slab tree itself actually boots fine. So Christoph's commit may be
> > merely disclosing some bug hidden in another for-next tree which
> > happens to be merged before the slab tree..
>
> Sorry: the bug does appear in the standalone slab tree, where the
> dmesg is
>
> [ 307.648802] blkid (2963) used greatest stack depth: 2832 bytes left
> [ 307.892070] vhci_hcd: changed 0
> [ 308.766647]
> [ 308.766648] =========================
> [ 308.766649] [ BUG: held lock freed! ]
> [ 308.766651] 3.5.0-rc1+ #44 Not tainted
> [ 308.766651] -------------------------
> [ 308.766653] mtd_probe/3040 is freeing memory ffff880006defdd0-ffff880006df0dcf, with a lock still held there!
> [ 308.766662] (&type->s_umount_key#31/1){+.+.+.}, at: [<ffffffff81187166>] sget+0x299/0x463
> [ 308.766663] 3 locks held by mtd_probe/3040:
> [ 308.766667] #0: (&type->s_umount_key#31/1){+.+.+.}, at: [<ffffffff81187166>] sget+0x299/0x463
> [ 308.766671] #1: (sb_lock){+.+.-.}, at: [<ffffffff81186f00>] sget+0x33/0x463
> [ 308.766675] #2: (unnamed_dev_lock){+.+...}, at: [<ffffffff81186711>] get_anon_bdev+0x38/0xe8
> [ 308.766675]
> [ 308.766675] stack backtrace:
> [ 308.766677] Pid: 3040, comm: mtd_probe Not tainted 3.5.0-rc1+ #44
> [ 308.766678] Call Trace:
> [ 308.766683] [<ffffffff810ddc6e>] debug_check_no_locks_freed+0x109/0x14b
> [ 308.766703] [<ffffffff81173f7c>] kmem_cache_free+0x2e/0xa7
> [ 308.766708] [<ffffffff816a5d9d>] ida_get_new_above+0x173/0x184
> [ 308.766711] [<ffffffff810db9a4>] ? lock_acquired+0x1e4/0x219
> [ 308.766713] [<ffffffff81186727>] get_anon_bdev+0x4e/0xe8
> [ 308.766715] [<ffffffff811867d8>] set_anon_super+0x17/0x2a
> [ 308.766717] [<ffffffff81187270>] sget+0x3a3/0x463
> [ 308.766719] [<ffffffff811867c1>] ? get_anon_bdev+0xe8/0xe8
> [ 308.766722] [<ffffffff811a1fbe>] mount_pseudo+0x31/0x152
> [ 308.766727] [<ffffffff81cb1f54>] mtd_inodefs_mount+0x24/0x26
> [ 308.766729] [<ffffffff81187e34>] mount_fs+0x69/0x155
> [ 308.766733] [<ffffffff811531b2>] ? __alloc_percpu+0x10/0x12
> [ 308.766736] [<ffffffff8119ca4c>] vfs_kern_mount+0x62/0xd9
> [ 308.766739] [<ffffffff811a1b43>] simple_pin_fs+0x4c/0x9b
> [ 308.766741] [<ffffffff81cb338a>] mtdchar_open+0x42/0x188
> [ 308.766744] [<ffffffff811886ef>] chrdev_open+0x11f/0x14a
> [ 308.766747] [<ffffffff810c0880>] ? local_clock+0x19/0x52
> [ 308.766750] [<ffffffff811885d0>] ? cdev_put+0x26/0x26
> [ 308.766752] [<ffffffff811836cc>] do_dentry_open+0x1e4/0x2b2
> [ 308.766754] [<ffffffff8118434a>] nameidata_to_filp+0x5e/0xa3
> [ 308.766756] [<ffffffff8119118f>] do_last+0x68f/0x6d3
> [ 308.766759] [<ffffffff811912d8>] path_openat+0xd2/0x32a
> [ 308.766762] [<ffffffff8111eed8>] ? time_hardirqs_off+0x26/0x2a
> [ 308.766765] [<ffffffff810d9e88>] ? trace_hardirqs_off+0xd/0xf
> [ 308.766767] [<ffffffff81191630>] do_filp_open+0x38/0x86
> [ 308.766771] [<ffffffff82e95e22>] ? _raw_spin_unlock+0x28/0x3b
> [ 308.766773] [<ffffffff8119baa7>] ? alloc_fd+0xe5/0xf7
> [ 308.766776] [<ffffffff811843fd>] do_sys_open+0x6e/0xfb
> [ 308.766777] [<ffffffff811844ab>] sys_open+0x21/0x23
> [ 308.766780] [<ffffffff82e9cb69>] system_call_fastpath+0x16/0x1b
Another dmesg on the slab tree:
[ 54.522438] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[ 54.567847] serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
[ 54.591289]
[ 54.591290] =========================
[ 54.591291] [ BUG: held lock freed! ]
[ 54.591293] 3.5.0-rc1+ #45 Not tainted
[ 54.591293] -------------------------
[ 54.591295] swapper/0/1 is freeing memory ffff88000f45fdd0-ffff88000f460dcf, with a lock still held there!
[ 54.591304] (&port->mutex){+.+.+.}, at: [<ffffffff817df6d2>] uart_add_one_port+0x84/0x356
[ 54.591306] 3 locks held by swapper/0/1:
[ 54.591310] #0: (port_mutex){+.+.+.}, at: [<ffffffff817df6c0>] uart_add_one_port+0x72/0x356
[ 54.591314] #1: (&port->mutex){+.+.+.}, at: [<ffffffff817df6d2>] uart_add_one_port+0x84/0x356
[ 54.591319] #2: (sysfs_ino_lock){+.+...}, at: [<ffffffff811e273f>] sysfs_new_dirent+0x6b/0x10c
[ 54.591320]
[ 54.591320] stack backtrace:
[ 54.591322] Pid: 1, comm: swapper/0 Not tainted 3.5.0-rc1+ #45
[ 54.591323] Call Trace:
[ 54.591338] [<ffffffff810ddc6e>] debug_check_no_locks_freed+0x109/0x14b
[ 54.591342] [<ffffffff81173fa0>] kmem_cache_free+0x2e/0xa7
[ 54.591346] [<ffffffff816a5d9d>] ida_get_new_above+0x173/0x184
[ 54.591351] [<ffffffff810db9a4>] ? lock_acquired+0x1e4/0x219
[ 54.591354] [<ffffffff811e2754>] sysfs_new_dirent+0x80/0x10c
[ 54.591357] [<ffffffff811e1cad>] sysfs_add_file_mode+0x4e/0xce
[ 54.591366] [<ffffffff811e1d3f>] sysfs_add_file+0x12/0x14
[ 54.591368] [<ffffffff811e4296>] sysfs_merge_group+0x45/0x97
[ 54.591372] [<ffffffff819bff1b>] dpm_sysfs_add+0x54/0xab
[ 54.591374] [<ffffffff819b9394>] device_add+0x3ba/0x5d7
[ 54.591377] [<ffffffff819b95cc>] device_register+0x1b/0x1f
[ 54.591379] [<ffffffff819b9661>] device_create_vargs+0x91/0xc8
[ 54.591381] [<ffffffff819b96c9>] device_create+0x31/0x33
[ 54.591385] [<ffffffff817c1792>] tty_register_device+0xde/0xfb
[ 54.591388] [<ffffffff817df90f>] uart_add_one_port+0x2c1/0x356
[ 54.591406] [<ffffffff84641c95>] serial8250_init+0x12b/0x189
[ 54.591409] [<ffffffff84641095>] ? r3964_init+0x25/0x41
[ 54.591411] [<ffffffff84641b6a>] ? serial8250_console_init+0x2c/0x2c
[ 54.591414] [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
[ 54.591419] [<ffffffff84603d39>] kernel_init+0x170/0x1f8
[ 54.591422] [<ffffffff84603590>] ? do_early_param+0x8c/0x8c
[ 54.591435] [<ffffffff82e9dfb4>] kernel_thread_helper+0x4/0x10
[ 54.591438] [<ffffffff82e961f0>] ? retint_restore_args+0x13/0x13
[ 54.591441] [<ffffffff84603bc9>] ? start_kernel+0x3e7/0x3e7
[ 54.591443] [<ffffffff82e9dfb0>] ? gs_change+0x13/0x13
[ 54.625113] 00:06: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: linux-next BUG: held lock freed!
From: Feng Tang @ 2012-07-02 4:28 UTC (permalink / raw)
To: Fengguang Wu, Christoph Lameter
Cc: Trond Myklebust, linux-nfs, linux-kernel, netdev, penberg,
linux-mm, Stephen Rothwell, bfields
In-Reply-To: <CA++bM2txX2f=SC3r3bwxLcB8CUCuELW-NhytrKW7-07kysfA2A@mail.gmail.com>
> From: Fengguang Wu <fengguang.wu@intel.com>
> Date: 2012/7/2
> Subject: linux-next BUG: held lock freed!
> To: Christoph Lameter <cl@linux.com>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>, "J. Bruce Fields" <
> bfields@fieldses.org>, linux-nfs@vger.kernel.org, LKML <
> linux-kernel@vger.kernel.org>, netdev <netdev@vger.kernel.org>, Pekka
> Enberg <penberg@kernel.org>, Linux Memory Management List <
> linux-mm@kvack.org>, Stephen Rothwell <sfr@canb.auug.org.au>
>
>
> Hi all,
>
> More observations on this bug:
>
> The slab tree itself actually boots fine. So Christoph's commit may be
> merely disclosing some bug hidden in another for-next tree which
> happens to be merged before the slab tree..
>
> Attached are some more back traces related to this bug (obviuosly, not
> only network locks are affected by the bug), as well as the kconfig that
> can relatively easily (but not always) trigger this bug.
>
> Thanks,
> Fengguang
>
> On Wed, Jun 27, 2012 at 08:23:06PM +0800, Fengguang Wu wrote:
> > Hi Christoph,
> >
> > It's a surprise that it bisects down to this commit. I confirmed
> > that it boots reliably if reverting this commit on top of linux-next.
> >
> > 8c138bc00925521c4e764269db3a903bd2a51592 is the first bad commit
> > commit 8c138bc00925521c4e764269db3a903bd2a51592
> > Author: Christoph Lameter <cl@linux.com>
> > Date: Wed Jun 13 10:24:58 2012 -0500
> >
> > slab: Get rid of obj_size macro
> >
> > The size of the slab object is frequently needed. Since we now
> > have a size field directly in the kmem_cache structure there is no
> > need anymore of the obj_size macro/function.
> >
> > Signed-off-by: Christoph Lameter <cl@linux.com>
> > Signed-off-by: Pekka Enberg <penberg@kernel.org>
Seems there is a typo in the original patch 8c138bc0:
@@ -3896,9 +3890,9 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
unsigned long flags;
local_irq_save(flags);
- debug_check_no_locks_freed(objp, obj_size(cachep));
+ debug_check_no_locks_freed(objp, cachep->size);
====> this should be cachep->object_size
if (!(cachep->flags & SLAB_DEBUG_OBJECTS))
- debug_check_no_obj_freed(objp, obj_size(cachep));
+ debug_check_no_obj_freed(objp, cachep->object_size);
So the following small patch may fix it:
----------------------
diff --git a/mm/slab.c b/mm/slab.c
index 64c3d03..605b3b7 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3890,7 +3890,7 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
unsigned long flags;
local_irq_save(flags);
- debug_check_no_locks_freed(objp, cachep->size);
+ debug_check_no_locks_freed(objp, cachep->object_size);
if (!(cachep->flags & SLAB_DEBUG_OBJECTS))
debug_check_no_obj_freed(objp, cachep->object_size);
__cache_free(cachep, objp, __builtin_return_address(0));
Thanks,
Feng
> >
> > :040000 040000 e0418be654b66b2364add59bb469024fd6958791
> f6be0da4d4740844ab8a4c561dbe3815a3f9b8b4 M mm
> > bisect run success
> >
> > > > [ 133.909702] =========================
> > > > [ 133.910694] [ BUG: held lock freed! ]
> > > > [ 133.911700] 3.5.0-rc4+ #5 Not tainted
> > > > [ 133.912672] -------------------------
> > > > [ 133.912969] swapper/0/0 is freeing memory
> ffff88001233ce08-ffff88001233de07, with a lock still held there!
> > > > [ 133.912969] (slock-AF_INET-RPC/1){+.-...}, at:
> [<ffffffff82ae84ee>] tcp_v4_rcv+0x28b/0x6fc
> > > > [ 133.912969] 3 locks held by swapper/0/0:
> > > > [ 133.912969] #0: (rcu_read_lock){.+.+..}, at:
> [<ffffffff82a1ea8a>] rcu_lock_acquire+0x0/0x29
> > > > [ 133.912969] #1: (rcu_read_lock){.+.+..}, at:
> [<ffffffff82aca483>] rcu_lock_acquire.constprop.14+0x0/0x30
> > > > [ 133.912969] #2: (slock-AF_INET-RPC/1){+.-...}, at:
> [<ffffffff82ae84ee>] tcp_v4_rcv+0x28b/0x6fc
> > > > [ 133.912969]
> > > > [ 133.912969] stack backtrace:
> > > > [ 133.912969] Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc4+ #5
> > > > [ 133.912969] Call Trace:
> > > > [ 133.912969] <IRQ> [<ffffffff810e09ae>]
> debug_check_no_locks_freed+0x109/0x14b
> > > > [ 133.912969] [<ffffffff811774e0>] kmem_cache_free+0x2e/0xa7
> > > > [ 133.912969] [<ffffffff82a191e5>] __kfree_skb+0x7f/0x83
> > > > [ 133.912969] [<ffffffff82adeccd>] tcp_ack+0x45d/0xc6a
> > > > [ 133.912969] [<ffffffff810c22ae>] ? local_clock+0x3b/0x52
> > > > [ 133.912969] [<ffffffff82adff44>] tcp_rcv_state_process+0x15a/0x7c6
> > > > [ 133.912969] [<ffffffff82ae79e7>] tcp_v4_do_rcv+0x341/0x390
> > > > [ 133.912969] [<ffffffff82ae88db>] tcp_v4_rcv+0x678/0x6fc
> > > > [ 133.912969] [<ffffffff82aca618>]
> ip_local_deliver_finish+0x165/0x1e4
> > > > [ 133.912969] [<ffffffff82acab4a>] ip_local_deliver+0x53/0x84
> > > > [ 133.912969] [<ffffffff810c228c>] ? local_clock+0x19/0x52
> > > > [ 133.912969] [<ffffffff82aca9c6>] ip_rcv_finish+0x32f/0x367
> > > > [ 133.912969] [<ffffffff82acad8b>] ip_rcv+0x210/0x269
> > > > [ 133.912969] [<ffffffff82a1eab1>] ? rcu_lock_acquire+0x27/0x29
> > > > [ 133.912969] [<ffffffff82a1ea8a>] ? softnet_seq_show+0x68/0x68
> > > > [ 133.912969] [<ffffffff82a21ede>] __netif_receive_skb+0x3cd/0x464
> > > > [ 133.912969] [<ffffffff82a21fda>] netif_receive_skb+0x65/0x9c
> > > > [ 133.912969] [<ffffffff82a227c5>] ? __napi_gro_receive+0xf2/0xff
> > > > [ 133.912969] [<ffffffff82a2209e>] napi_skb_finish+0x26/0x58
> > > > [ 133.912969] [<ffffffff810c228c>] ? local_clock+0x19/0x52
> > > > [ 133.912969] [<ffffffff82a228c5>] napi_gro_receive+0x2f/0x34
> > > > [ 133.912969] [<ffffffff81e36d12>] e1000_receive_skb+0x57/0x60
> > > > [ 133.912969] [<ffffffff81e39b23>] e1000_clean_rx_irq+0x2f2/0x387
> > > > [ 133.912969] [<ffffffff81e390f3>] e1000_clean+0x541/0x695
> > > > [ 133.912969] [<ffffffff8106c57b>] ? kvm_clock_read+0x2e/0x36
> > > > [ 133.912969] [<ffffffff82a22402>] ? net_rx_action+0x1b3/0x1f8
> > > > [ 133.912969] [<ffffffff82a22302>] net_rx_action+0xb3/0x1f8
> > > > [ 133.912969] [<ffffffff810984ab>] ? __do_softirq+0x76/0x1e8
> > > > [ 133.912969] [<ffffffff81098515>] __do_softirq+0xe0/0x1e8
> > > > [ 133.912969] [<ffffffff81122190>] ? time_hardirqs_off+0x26/0x2a
> > > > [ 133.912969] [<ffffffff82ea7fec>] call_softirq+0x1c/0x30
> > > > [ 133.912969] [<ffffffff81049cc8>] do_softirq+0x4a/0xa2
> > > > [ 133.912969] [<ffffffff8109888e>] irq_exit+0x51/0xbc
> > > > [ 133.912969] [<ffffffff82ea88ae>] do_IRQ+0x8e/0xa5
> > > > [ 133.912969] [<ffffffff82ea002f>] common_interrupt+0x6f/0x6f
> > > > [ 133.912969] <EOI> [<ffffffff8106c76b>] ? native_safe_halt+0x6/0x8
> > > > [ 133.912969] [<ffffffff810e08a3>] ? trace_hardirqs_on+0xd/0xf
> > > > [ 133.912969] [<ffffffff8104f384>] default_idle+0x53/0x90
> > > > [ 133.912969] [<ffffffff8104fc09>] cpu_idle+0xcc/0x123
> > > > [ 133.912969] [<ffffffff82d1d8dd>] rest_init+0xd1/0xda
> > > > [ 133.912969] [<ffffffff82d1d80c>] ?
> csum_partial_copy_generic+0x16c/0x16c
> > > > [ 133.912969] [<ffffffff8460dbbc>] start_kernel+0x3da/0x3e7
> > > > [ 133.912969] [<ffffffff8460d5ea>] ? repair_env_string+0x5a/0x5a
> > > > [ 133.912969] [<ffffffff8460d2d6>]
> x86_64_start_reservations+0xb1/0xb5
> > > > [ 133.912969] [<ffffffff8460d3d8>] x86_64_start_kernel+0xfe/0x10b
> > > > [ 134.024230] VFS: Mounted root (nfs filesystem) on device 0:14.
> >
> > Thanks,
> > Fengguang
> > --
> > 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/
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: AF_BUS socket address family
From: Chris Friesen @ 2012-07-02 4:49 UTC (permalink / raw)
To: David Miller; +Cc: vincent.sanders, netdev, linux-kernel
In-Reply-To: <20120629.161821.948325645333976311.davem@davemloft.net>
On 06/29/2012 05:18 PM, David Miller wrote:
> From: Vincent Sanders<vincent.sanders@collabora.co.uk>
> Date: Sat, 30 Jun 2012 00:12:37 +0100
>
>> I had hoped you would have at least read the opening list where I
>> outlined the underlying features which explain why none of the
>> existing IPC match the requirements.
> I had hoped that you had read the part we told you last time where
> we explained why multicast and "reliable delivery" are fundamentally
> incompatible attributes.
>
> We are not creating a full address family in the kernel which exists
> for one, and only one, specific and difficult user.
For what it's worth, the company I work for (and a number of other
companies) currently use an out-of-tree datagram multicast messaging
protocol family based on AF_UNIX.
If AF_BUS were to be accepted, it would be essentially trivial for us to
port our existing userspace messaging library to use it instead of our
current protocol family, and we would almost certainly do so.
I'd love to see AF_BUS go in.
Chris Friesen
^ permalink raw reply
* Re: [PATCH net-next 00/11] default maximal number of RSS queues in mq drivers
From: Yuval Mintz @ 2012-07-02 4:55 UTC (permalink / raw)
To: Or Gerlitz
Cc: davem, netdev, eilong, Divy Le Ray, Jon Mason,
Anirban Chakraborty, Jitendra Kalsaria, Ron Mercer, Jeff Kirsher,
Jon Mason, Andrew Gallatin, Sathya Perla, Subbu Seetharaman,
Ajit Khaparde, Matt Carlson, Michael Chan, Eric Dumazet,
Ben Hutchings
In-Reply-To: <4FF0663C.1070306@mellanox.com>
> I assume this posting is actually v3 of "[RFC net-next (v2) 00/14] default maximal number of RSS queues in mq drivers" see http://marc.info/?l=linux-netdev&m=134062509310404&w=2?
>
Of course you are correct - the fact that it's based on that RFC is stated
explicitly in PATCH 0 of this series.
> correct? so what has been changed along v0/v1/v2 --> v3? It would be very helpful to follow
> if you make sure to mark the patches that makes Vn with "PATCH net-next Vn" and specify the changes
> from Vn-1, Vn-2...V1, V0.
Certainly -
Changes from RFC v2:
- Removed changes for igb, igbxe, and igxbevf drivers.
Changes from RFC v1:
- Use 'netif_get_num_default_rss_queues()' wrapper function instead
of using a hard-coded define value directly.
- Corrected be2net implementation of this fix.
Cheers,
Yuval
^ permalink raw reply
* Re: [PATCH net-next] em_canid: Ematch rule to match CAN frames according to their identifiers
From: Oliver Hartkopp @ 2012-07-02 5:04 UTC (permalink / raw)
To: Rostislav Lisovy; +Cc: netdev, linux-can, lartc, pisa, sojkam1
In-Reply-To: <4FEDCD42.8010203@hartkopp.net>
One additional remark:
When applying your patch on Dave Millers net-next tree there are some offsets
in the patch ...
Please send the next patch based on the net-next tree, as this would be the
tree, it will be applied to. See URLs at:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git
Tnx!
Oliver
On 29.06.2012 17:44, Oliver Hartkopp wrote:
> Hello Rostislav,
>
> looks really good now.
>
> 1. Your Signed-off-by: is missing.
>
> 2. One remark to a removed length check:
>
> (..)
>
>> +static int em_canid_change(struct tcf_proto *tp, void *data, int len,
>> + struct tcf_ematch *m)
>> +{
>> + struct can_filter *conf = data; /* Array with rules,
>> + * fixed size EM_CAN_RULES_SIZE
>> + */
>> + struct canid_match *cm;
>> + struct canid_match *cm_old = (struct canid_match *) m->data;
>> + int i;
>> + int rulescnt;
>> +
>
>
> What about a zero length check here?
>
> if (!len)
> return -EINVAL;
>
> ???
>
>> + if (len % sizeof(struct can_filter))
>> + return -EINVAL;
>> +
>> + if (len > sizeof(struct can_filter) * EM_CAN_RULES_MAX)
>> + return -EINVAL;
>> +
>> + rulescnt = len / sizeof(struct can_filter);
>> +
>> + cm = kzalloc(sizeof(struct canid_match) + sizeof(struct can_filter) *
>> + rulescnt, GFP_KERNEL);
>> + if (!cm)
>> + return -ENOMEM;
>
>
> The length could alternatively be checked here too
>
> http://lxr.linux.no/#linux+v3.4.4/net/sched/ematch.c#L235
>
> if em->ops->datalen is set.
>
> But here's no
>
> .datalen = sizeof(struct can_filter),
>
> defined, right?
>
>> +static struct tcf_ematch_ops em_canid_ops = {
>> + .kind = TCF_EM_CANID,
>> + .change = em_canid_change,
>> + .match = em_canid_match,
>> + .destroy = em_canid_destroy,
>> + .dump = em_canid_dump,
>> + .owner = THIS_MODULE,
>> + .link = LIST_HEAD_INIT(em_canid_ops.link)
>> +};
>
>
> Regards,
> Oliver
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: possible integer underflow in __sctp_auth_cid()
From: Wei Yongjun @ 2012-07-02 5:06 UTC (permalink / raw)
To: dan.carpenter; +Cc: vyasevich, linux-sctp, netdev
On 06/30/2012 08:17 PM, Dan Carpenter wrote:
> In 555d3d5d "SCTP: Fix chunk acceptance when no authenticated chunks
> were listed.", we added a check for if (param->param_hdr.length == 0).
> Shouldn't that check be a check for if
> (param->param_hdr.length < sizeos(sizeof(sctp_paramhdr_t)))? Otherwise,
> when we do the substraction on the next line we would unintentionally
> end up with a high positive number.
>
> I had a similar question about sctp_auth_ep_add_chunkid():
>
> net/sctp/auth.c
> 770 /* Check if we can add this chunk to the array */
> 771 param_len = ntohs(p->param_hdr.length);
> 772 nchunks = param_len - sizeof(sctp_paramhdr_t);
> 773 if (nchunks == SCTP_NUM_CHUNK_TYPES)
> 774 return -EINVAL;
> 775
> 776 p->chunks[nchunks] = chunk_id;
>
> If param_len is less than sizeof(sctp_paramhdr_t) we could write past
> the end of the array. There are a couple other places with this same
> subtraction as well.
This will not happen because that p which means ep->auth_chunk_list
is maintained by the endpoint itself, and the default value is
sizeof(sctp_paramhdr_t) or sizeof(sctp_paramhdr_t) + 2.
Instead, I found that if user enable AUTH after the endpoint is
created, set the AUTH chunk may cause panic, because the
ep->auth_chunk_list is NULL.
I think we should introduce a help function to check whether the
AUTH is enabled instead only check sctp_auth_enable, may like:
bool sctp_auth_ep_enabled(struct sctp_endpoint *ep)
{
if (!sctp_auth_enable)
return false;
if (!ep->auth_chunk_list || !ep->auth_hmacs_list)
return false;
return true;
}
Regards
Yongjun Wei
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Re: linux-next BUG: held lock freed!
From: Fengguang Wu @ 2012-07-02 5:22 UTC (permalink / raw)
To: Feng Tang
Cc: Christoph Lameter, Trond Myklebust, linux-nfs, linux-kernel,
netdev, penberg, linux-mm, Stephen Rothwell, bfields
In-Reply-To: <20120702122858.029946db@feng-i7>
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3890,7 +3890,7 @@ void kmem_cache_free(struct kmem_cache *cachep, void *objp)
> unsigned long flags;
>
> local_irq_save(flags);
> - debug_check_no_locks_freed(objp, cachep->size);
> + debug_check_no_locks_freed(objp, cachep->object_size);
> if (!(cachep->flags & SLAB_DEBUG_OBJECTS))
> debug_check_no_obj_freed(objp, cachep->object_size);
> __cache_free(cachep, objp, __builtin_return_address(0));
It works! No single error after a dozen reboots :-)
Tested-by: Fengguang Wu <wfg@linux.intel.com>
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net-next 1/2] ipv6: remove unnecessary codes in tcp_ipv6.c
From: RongQing Li @ 2012-07-02 5:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120701.202610.12425223200611171.davem@davemloft.net>
2012/7/2 David Miller <davem@davemloft.net>:
> From: roy.qing.li@gmail.com
> Date: Mon, 2 Jul 2012 11:18:59 +0800
>
>> - if (opt) {
>> - newnp->opt = ipv6_dup_options(newsk, opt);
>> - if (opt != np->opt)
>> - sock_kfree_s(sk, opt, opt->tot_len);
>
> This is bogus, if we copy the options into a new copy in
> ipv6_dup_options() we have to free the old one or else we
> leak it.
Do you mean I should free newnp->opt firstly ?
If I understand it right, I think we do not need to free it. the
process is below:
newsk = tcp_v4_syn_recv_sock(sk, skb, req, dst);
..
newnp = inet6_sk(newsk);
..
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
..
newnp->opt = NULL;
So newnp->opt is not a effective memory.
Thanks.
-Roy
^ permalink raw reply
* Re: [PATCH net-next 1/2] ipv6: remove unnecessary codes in tcp_ipv6.c
From: David Miller @ 2012-07-02 5:37 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <CAJFZqHxyR0PErHV8cqLLe6eqL0fCYS9gjPRc9zpthpevsU7wkA@mail.gmail.com>
From: RongQing Li <roy.qing.li@gmail.com>
Date: Mon, 2 Jul 2012 13:23:09 +0800
> 2012/7/2 David Miller <davem@davemloft.net>:
>> From: roy.qing.li@gmail.com
>> Date: Mon, 2 Jul 2012 11:18:59 +0800
>>
>>> - if (opt) {
>>> - newnp->opt = ipv6_dup_options(newsk, opt);
>>> - if (opt != np->opt)
>>> - sock_kfree_s(sk, opt, opt->tot_len);
>>
>> This is bogus, if we copy the options into a new copy in
>> ipv6_dup_options() we have to free the old one or else we
>> leak it.
>
> Do you mean I should free newnp->opt firstly ?
>
> If I understand it right, I think we do not need to free it. the
> process is below:
>
> newsk = tcp_v4_syn_recv_sock(sk, skb, req, dst);
> ..
> newnp = inet6_sk(newsk);
> ..
> memcpy(newnp, np, sizeof(struct ipv6_pinfo));
> ..
> newnp->opt = NULL;
>
> So newnp->opt is not a effective memory.
ipv6_dup_options() allocates new memory for the options and this call
statement assigns that new pointer to np->opt.
If you do not free the old (before ipv6_dup_options()) np->opt memory
here, it is lost forever.
^ permalink raw reply
* [PATCH net-next] 6lowpan: revert 'reuse eth_mac_addr()'
From: Alexander Smirnov @ 2012-07-02 5:58 UTC (permalink / raw)
To: davem; +Cc: netdev, danny.kukawka, dbaryshkov, Alexander Smirnov
This reverts the commit cdf49c283e2e105da86ca575ad35b453f5ff24ea which
replaces lowpan '.ndo_set_mac_address' method by ethernet's one.
Accorind to the IEEE 802.15.4 standard, device has 8-byte length address,
so this hook loses the last 2 bytes which may rise a compatibility problems
with other IEEE 802.15.4 standard implementations.
Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
net/ieee802154/6lowpan.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index ad0c226..049d8cd 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -55,7 +55,6 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/netdevice.h>
-#include <linux/etherdevice.h>
#include <net/af_ieee802154.h>
#include <net/ieee802154.h>
#include <net/ieee802154_netdev.h>
@@ -936,6 +935,19 @@ drop:
return -EINVAL;
}
+static int lowpan_set_address(struct net_device *dev, void *p)
+{
+ struct sockaddr *sa = p;
+
+ if (netif_running(dev))
+ return -EBUSY;
+
+ /* TODO: validate addr */
+ memcpy(dev->dev_addr, sa->sa_data, dev->addr_len);
+
+ return 0;
+}
+
static int lowpan_get_mac_header_length(struct sk_buff *skb)
{
/*
@@ -1078,7 +1090,7 @@ static struct header_ops lowpan_header_ops = {
static const struct net_device_ops lowpan_netdev_ops = {
.ndo_start_xmit = lowpan_xmit,
- .ndo_set_mac_address = eth_mac_addr,
+ .ndo_set_mac_address = lowpan_set_address,
};
static struct ieee802154_mlme_ops lowpan_mlme = {
--
1.7.2.3
^ permalink raw reply related
* [PATCH net-next 0/0] fix sparse warnings
From: Alexander Smirnov @ 2012-07-02 6:18 UTC (permalink / raw)
To: davem; +Cc: netdev, dbaryshkov
Hello David,
I've received a notification from Fengguang Wu that there are new sparse
warnings are shown up after my patch sets.
Completely forgot about 'sparse', aghhr...
These patches fixes the following sparse warnings:
- drivers/ieee802154/at86rf230.c:610:2: warning:
'rc' may be used uninitialized in this function [-Wuninitialized]
- net/ieee802154/6lowpan.c:127:12: sparse:
symbol 'flist_lock' was not declared. Should it be static?
- net/mac802154/mac_cmd.c:58:17: warning:
symbol 'mac802154_get_phy' was not declared. Should it be static?
- net/mac802154/mib.c:42:23: warning:
symbol 'mac802154_slave_get_priv' was not declared. Should it be static?
With best regards,
Alexander Smirnov
Alexander Smirnov (2):
ieee802154: sparse warnings: make symbols static
drivers/ieee802154/at231rf230: remove unused return status
drivers/ieee802154/at86rf230.c | 3 +--
net/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
--
1.7.2.3
^ permalink raw reply
* [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Alexander Smirnov @ 2012-07-02 6:18 UTC (permalink / raw)
To: davem; +Cc: netdev, dbaryshkov, Alexander Smirnov
In-Reply-To: <1341209912-6030-1-git-send-email-alex.bluesman.smirnov@gmail.com>
Make symbols static to avoid the following warning shown up
by sparse:
warning: symbol ... was not declared. Should it be static?
Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
net/ieee802154/6lowpan.c | 2 +-
net/mac802154/mac_cmd.c | 2 +-
net/mac802154/mib.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index cd5007f..17ad28f 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -124,7 +124,7 @@ struct lowpan_fragment {
static unsigned short fragment_tag;
static LIST_HEAD(lowpan_fragments);
-spinlock_t flist_lock;
+static spinlock_t flist_lock;
static inline struct
lowpan_dev_info *lowpan_dev_info(const struct net_device *dev)
diff --git a/net/mac802154/mac_cmd.c b/net/mac802154/mac_cmd.c
index 7f5403e..e6659fa 100644
--- a/net/mac802154/mac_cmd.c
+++ b/net/mac802154/mac_cmd.c
@@ -55,7 +55,7 @@ static int mac802154_mlme_start_req(struct net_device *dev,
return 0;
}
-struct wpan_phy *mac802154_get_phy(const struct net_device *dev)
+static struct wpan_phy *mac802154_get_phy(const struct net_device *dev)
{
struct mac802154_sub_if_data *priv = netdev_priv(dev);
diff --git a/net/mac802154/mib.c b/net/mac802154/mib.c
index 380829d..9cfa5f3 100644
--- a/net/mac802154/mib.c
+++ b/net/mac802154/mib.c
@@ -39,7 +39,7 @@ struct hw_addr_filt_notify_work {
unsigned long changed;
};
-struct mac802154_priv *mac802154_slave_get_priv(struct net_device *dev)
+static struct mac802154_priv *mac802154_slave_get_priv(struct net_device *dev)
{
struct mac802154_sub_if_data *priv = netdev_priv(dev);
--
1.7.2.3
^ permalink raw reply related
* [PATCH net-next 2/2] drivers/ieee802154/at231rf230: remove unused return status
From: Alexander Smirnov @ 2012-07-02 6:18 UTC (permalink / raw)
To: davem; +Cc: netdev, dbaryshkov, Alexander Smirnov
In-Reply-To: <1341209912-6030-1-git-send-email-alex.bluesman.smirnov@gmail.com>
Remove excessive variable used for the return status.
Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
---
drivers/ieee802154/at86rf230.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/ieee802154/at86rf230.c b/drivers/ieee802154/at86rf230.c
index 4d033d4..902e38b 100644
--- a/drivers/ieee802154/at86rf230.c
+++ b/drivers/ieee802154/at86rf230.c
@@ -585,7 +585,6 @@ err:
static int at86rf230_rx(struct at86rf230_local *lp)
{
u8 len = 128, lqi = 0;
- int rc;
struct sk_buff *skb;
skb = alloc_skb(len, GFP_KERNEL);
@@ -607,7 +606,7 @@ static int at86rf230_rx(struct at86rf230_local *lp)
ieee802154_rx_irqsafe(lp->dev, skb, lqi);
- dev_dbg(&lp->spi->dev, "READ_FBUF: %d %d %x\n", rc, len, lqi);
+ dev_dbg(&lp->spi->dev, "READ_FBUF: %d %x\n", len, lqi);
return 0;
err:
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Eric Dumazet @ 2012-07-02 6:37 UTC (permalink / raw)
To: Alexander Smirnov; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <1341209912-6030-2-git-send-email-alex.bluesman.smirnov@gmail.com>
On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
> Make symbols static to avoid the following warning shown up
> by sparse:
>
> warning: symbol ... was not declared. Should it be static?
>
> Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
> ---
> net/ieee802154/6lowpan.c | 2 +-
> net/mac802154/mac_cmd.c | 2 +-
> net/mac802154/mib.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> index cd5007f..17ad28f 100644
> --- a/net/ieee802154/6lowpan.c
> +++ b/net/ieee802154/6lowpan.c
> @@ -124,7 +124,7 @@ struct lowpan_fragment {
>
> static unsigned short fragment_tag;
> static LIST_HEAD(lowpan_fragments);
> -spinlock_t flist_lock;
> +static spinlock_t flist_lock;
>
static DEFINE_SPINLOCK(flist_lock);
^ permalink raw reply
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Eric Dumazet @ 2012-07-02 6:44 UTC (permalink / raw)
To: Alexander Smirnov; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <1341211072.5269.0.camel@edumazet-glaptop>
On Mon, 2012-07-02 at 08:37 +0200, Eric Dumazet wrote:
> On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
> > Make symbols static to avoid the following warning shown up
> > by sparse:
> >
> > warning: symbol ... was not declared. Should it be static?
> >
> > Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
> > ---
> > net/ieee802154/6lowpan.c | 2 +-
> > net/mac802154/mac_cmd.c | 2 +-
> > net/mac802154/mib.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> > index cd5007f..17ad28f 100644
> > --- a/net/ieee802154/6lowpan.c
> > +++ b/net/ieee802154/6lowpan.c
> > @@ -124,7 +124,7 @@ struct lowpan_fragment {
> >
> > static unsigned short fragment_tag;
> > static LIST_HEAD(lowpan_fragments);
> > -spinlock_t flist_lock;
> > +static spinlock_t flist_lock;
> >
>
> static DEFINE_SPINLOCK(flist_lock);
and of course commit 768f7c7c121e80f4 (6lowpan: add missing
spin_lock_init() ) must be reverted.
^ permalink raw reply
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Eric Dumazet @ 2012-07-02 6:49 UTC (permalink / raw)
To: Alexander Smirnov; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <1341211476.5269.2.camel@edumazet-glaptop>
On Mon, 2012-07-02 at 08:44 +0200, Eric Dumazet wrote:
> On Mon, 2012-07-02 at 08:37 +0200, Eric Dumazet wrote:
> > On Mon, 2012-07-02 at 10:18 +0400, Alexander Smirnov wrote:
> > > Make symbols static to avoid the following warning shown up
> > > by sparse:
> > >
> > > warning: symbol ... was not declared. Should it be static?
> > >
> > > Signed-off-by: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
> > > ---
> > > net/ieee802154/6lowpan.c | 2 +-
> > > net/mac802154/mac_cmd.c | 2 +-
> > > net/mac802154/mib.c | 2 +-
> > > 3 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> > > index cd5007f..17ad28f 100644
> > > --- a/net/ieee802154/6lowpan.c
> > > +++ b/net/ieee802154/6lowpan.c
> > > @@ -124,7 +124,7 @@ struct lowpan_fragment {
> > >
> > > static unsigned short fragment_tag;
> > > static LIST_HEAD(lowpan_fragments);
> > > -spinlock_t flist_lock;
> > > +static spinlock_t flist_lock;
> > >
> >
> > static DEFINE_SPINLOCK(flist_lock);
>
>
> and of course commit 768f7c7c121e80f4 (6lowpan: add missing
> spin_lock_init() ) must be reverted.
You should validate this code with LOCKDEP
lowpan_dellink() does a spin_lock(&flist_lock);
while same lock can be taken by lowpan_fragment_timer_expired() from
timer irq, -> deadlock.
del_timer() probably needs a del_timer_sync() too
^ permalink raw reply
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Alexander Smirnov @ 2012-07-02 6:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <1341211751.5269.6.camel@edumazet-glaptop>
Dear Eric,
>> > static DEFINE_SPINLOCK(flist_lock);
>>
>>
>> and of course commit 768f7c7c121e80f4 (6lowpan: add missing
>> spin_lock_init() ) must be reverted.
>
> You should validate this code with LOCKDEP
>
> lowpan_dellink() does a spin_lock(&flist_lock);
> while same lock can be taken by lowpan_fragment_timer_expired() from
> timer irq, -> deadlock.
>
> del_timer() probably needs a del_timer_sync() too
>
Thanks a lot for the hints!
Alex
^ permalink raw reply
* AW: RFC: replace packets already in queue
From: Erdt, Ralph @ 2012-07-02 7:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Rick Jones, netdev@vger.kernel.org
In-Reply-To: <1340960817.15719.6.camel@edumazet-glaptop>
Hello Eric Dumazet,
sorry for the late answer, but sometimes it's not easy here...
> Problem is : with wireless, chances are high that the old packet is not
> waiting in qdisc, but in wireless queues.
Even if the wireless queue is a problem (because of our setup, this is not a problem), the network stack queue (*) is the biggest queue, and a good point to optimize. And its impossible doing things 100%. We are happy to get 99%. This is a great improvement compared to the previous situation.
(* When I'm getting the picture in http://lartc.org/howto/lartc.qdisc.terminology.html correct, the qdisc is the outgoing queue and the last queue of the kernel. After this queue, there are only the hardware queues.)
> Anyway, adding a maxdelay to codel / fq_codel is really easy : This
> would drop packet if its sejourn time is above a given limit.
Just dropping when to old is not our goal. In fact we are avoiding blind dropping! With our replace we make sure that a packet of one class will go over the air when this packet is in line. But the information was replaced during the stay in the queue, so that the newest information got sent.
> If you want I can cook this patch.
Thank for you offer/support. But as described above, this is not a solution for us (we discussed this again in our group).
Greetings
Ralph Erdt
^ permalink raw reply
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Eric Dumazet @ 2012-07-02 7:09 UTC (permalink / raw)
To: Alexander Smirnov; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <CAJmB2rDB6McXg=O_z-M3xFa0OrB5hkTWT=ZHM975GDTPvuGjtQ@mail.gmail.com>
On Mon, 2012-07-02 at 10:53 +0400, Alexander Smirnov wrote:
> Dear Eric,
>
> >> > static DEFINE_SPINLOCK(flist_lock);
> >>
> >>
> >> and of course commit 768f7c7c121e80f4 (6lowpan: add missing
> >> spin_lock_init() ) must be reverted.
> >
> > You should validate this code with LOCKDEP
> >
> > lowpan_dellink() does a spin_lock(&flist_lock);
> > while same lock can be taken by lowpan_fragment_timer_expired() from
> > timer irq, -> deadlock.
> >
> > del_timer() probably needs a del_timer_sync() too
> >
>
> Thanks a lot for the hints!
While you are changing this code, please add in
lowpan_alloc_new_frame() :
- use netdev_alloc_skb_ip_align() instead of alloc_skb() to get some
extra headroom in case we need to forward this frame in a tunnel or
something...
- initialize frame->lock
^ permalink raw reply
* Re: [PATCH net-next 1/2] ieee802154: sparse warnings: make symbols static
From: Eric Dumazet @ 2012-07-02 7:13 UTC (permalink / raw)
To: Alexander Smirnov; +Cc: davem, netdev, dbaryshkov
In-Reply-To: <1341212942.5269.10.camel@edumazet-glaptop>
On Mon, 2012-07-02 at 09:09 +0200, Eric Dumazet wrote:
> mething...
>
> - initialize frame->lock
or better, remove lock from struct lowpan_fragment
^ permalink raw reply
* RE: [PATCH net-next 2/2] r8169: support RTL8168G
From: hayeswang @ 2012-07-02 7:27 UTC (permalink / raw)
To: 'Francois Romieu'; +Cc: netdev, linux-kernel
In-Reply-To: <20120629135111.GB8560@electric-eye.fr.zoreil.com>
[...]
> > +static void r8168_phy_ocp_write(void __iomem *ioaddr, u32
> reg, u32 data)
> > +{
> > + int i;
> > +
> > + if (reg & 0xffff0001)
> > + BUG();
>
> The patch adds a lot of BUG(). BUG is terrible from a system
> or end user
> viewpoint.
>
> Were they only a devel helper or are they still supposed to be of use
> in the future ? If the latter applies, why ?
>
I think if the reg is invalid, there must be something wrong. The code should
have bug, and I should notice develper or someone using the code. I would
replace them with showing messages.
[...]
> > +static void rtl_ocp_write_fw(struct rtl8169_private *tp,
> struct rtl_fw *rtl_fw)
> > +{
> > + struct rtl_fw_phy_action *pa = &rtl_fw->phy_action;
> > + void __iomem *ioaddr = tp->mmio_addr;
> > + u32 predata, count;
> > + u32 base_addr;
> > + size_t index;
> > +
> > + predata = count = 0;
> > + base_addr = 0xa400;
> > +
> > + for (index = 0; index < pa->size; ) {
> > + u32 action = le32_to_cpu(pa->code[index]);
> > + u32 data = action & 0x0000ffff;
> > + u32 regno = (action & 0x0fff0000) >> 16;
> > +
> > + if (!action)
> > + break;
> > +
> > + switch(action & 0xf0000000) {
> > + case PHY_READ:
> > + predata = r8168_phy_ocp_read(ioaddr,
> > + base_addr + (regno -16) * 2);
> > + count++;
> > + index++;
> > + break;
> [duplicated code removed]
> > + case PHY_WRITE:
> > + if (regno == 0x1f)
> > + base_addr = data << 4;
> > + else
> > + r8168_phy_ocp_write(ioaddr,
> > + base_addr +
> (regno - 0x10) * 2,
> > + data);
> > + index++;
> > + break;
> [duplicated code removed]
> > + case PHY_WRITE_PREVIOUS:
> > + r8168_phy_ocp_write(ioaddr, base_addr +
> (regno -16) * 2,
> > + predata);
> > + index++;
> > + break;
>
> I can't believe that the hardware people have designed something which
> needs a different firmware write method, especially as it
> copies at lot
> of code.
>
> How did you come to the conclusion that it was not possible
> to hide this
> stuff behind r8168g_mdio_{read / write} ?
>
> I would not mind replacing the
> PHY_{READ/WRITE/WRITE_PREVIOUS} case with
> chipset specific {READ/WRITE/WRITE_PREVIOUS} methods as long as the
> semantic looks the same but going through a different
> (*write_fw) does not
> trivially seem to be the best abstraction.
>
The difficulty is how to deal with the base_addr. Although it should not happen,
the base_addr may be modify if two threads access phy at the same time. I would
replace the method by saving the base_addr with a global variable. Then, the
r8168g_mdio functions could deal with both of the firmware and phy settings.
[...]
> > +static void __devinit rtl_hw_initialize(struct rtl8169_private *tp)
> > +{
> > + switch (tp->mac_version) {
> > + case RTL_GIGA_MAC_VER_40:
> > + case RTL_GIGA_MAC_VER_41:
> > + rtl_hw_init_8168g(tp);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
>
> Why doesn't it belong to hw_start ?
>
According to the initialization process from our hardware, there are something
needed to do before reset. Maybe this considers the rebooting from the other OS
or hwardware behavior. I don't sure if it is safe, to let them belong to
hw_start.
> Is it completely unneeded if the device requires a rtl8169_hw_reset,
> resumes or such ?
>
By the information from our hardware, these are the initial settings and only
need be done once.
Best Regards,
Hayes
^ permalink raw reply
* Re: AW: RFC: replace packets already in queue
From: Eric Dumazet @ 2012-07-02 7:31 UTC (permalink / raw)
To: Erdt, Ralph; +Cc: Rick Jones, netdev@vger.kernel.org
In-Reply-To: <FB112703C4930F4ABEBB5B763F964911393795D6@MAILSERV2A.lorien.fkie.fgan.de>
On Mon, 2012-07-02 at 07:02 +0000, Erdt, Ralph wrote:
> Even if the wireless queue is a problem (because of our setup, this is
> not a problem), the network stack queue (*) is the biggest queue, and
> a good point to optimize.
Hmm, I am not convinced you have no queues on wireless.
Please describe how you managed this.
In fact this is the biggest problem with wireless : mac82011 framework
aggressively pull packets from Linux packet qdisc in order to perform
packet aggregation.
Most packets don't stay in qdisc but are sitting in wireless driver,
unless you really flood it. If it happens, you already are in trouble.
So code your qdisc thing, but I am not sure you'll get much improvement.
You would need to implement it in wireless code instead.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox