Message ID | 1415297355-27282-1-git-send-email-anish@chelsio.com |
---|---|
State | Deferred, archived |
Delegated to: | David Miller |
Headers | show |
On 11/06/2014 10:09 AM, Anish Bhatt wrote: > dcb_lock was being used uninitialized in dcbnl and is infact missing > initialization code. Fixed > Are you trying to resolve a bug? It is initialized with static DEFINE_SPINLOCK(dcb_lock); and if you follow the code far enough you get to this in spinlock_types.h: #ifdef CONFIG_DEBUG_SPINLOCK # define SPIN_DEBUG_INIT(lockname) \ .magic = SPINLOCK_MAGIC, \ .owner_cpu = -1, \ .owner = SPINLOCK_OWNER_INIT, #else # define SPIN_DEBUG_INIT(lockname) #endif #define __RAW_SPIN_LOCK_INITIALIZER(lockname) \ { \ .raw_lock = __ARCH_SPIN_LOCK_UNLOCKED, \ SPIN_DEBUG_INIT(lockname) \ SPIN_DEP_MAP_INIT(lockname) } [...]
Yes, without this kernel is complaining about inconsitent lock state when lock debugging is enabled. Unfortunately I do not have the trace lying around right now. If you wish, you can reject this patch, I'll resend it when I get the trace again, with trace included. -Anish
On 11/06/2014 11:12 AM, Anish Bhatt wrote: > Yes, without this kernel is complaining about inconsitent lock state > when lock debugging is enabled. Unfortunately I do not have the trace > lying around right now. > If you have the trace that might help. I can't recall seeing any splats in these code paths. Also as far as I can tell you shouldn't need to do an init after the define. There are lots of examples in ./net/core where this is done. So we need to sort out why the init resolves the issue. > If you wish, you can reject this patch, I'll resend it when I get the trace again, with trace included. > -Anish > ________________________________________ > From: John Fastabend [john.fastabend@gmail.com] > Sent: Thursday, November 06, 2014 11:03 AM > To: Anish Bhatt > Cc: netdev@vger.kernel.org; davem@davemloft.net; john.r.fastabend@intel.com; ying.xue@windriver.com; jeffrey.t.kirsher@intel.com; ebiederm@xmission.com > Subject: Re: [PATCH net] dcbnl : Fix lock initialization > > On 11/06/2014 10:09 AM, Anish Bhatt wrote: >> dcb_lock was being used uninitialized in dcbnl and is infact missing >> initialization code. Fixed >> > > Are you trying to resolve a bug? It is initialized with > > static DEFINE_SPINLOCK(dcb_lock); > > and if you follow the code far enough you get to this in > spinlock_types.h: > > > #ifdef CONFIG_DEBUG_SPINLOCK > # define SPIN_DEBUG_INIT(lockname) \ > .magic = SPINLOCK_MAGIC, \ > .owner_cpu = -1, \ > .owner = SPINLOCK_OWNER_INIT, > #else > # define SPIN_DEBUG_INIT(lockname) > #endif > > #define __RAW_SPIN_LOCK_INITIALIZER(lockname) \ > { \ > .raw_lock = __ARCH_SPIN_LOCK_UNLOCKED, \ > SPIN_DEBUG_INIT(lockname) \ > SPIN_DEP_MAP_INIT(lockname) } > > [...] > > > > -- > John Fastabend Intel Corporation >
Dave, Please do not apply this patch then. John, I will try to recreate this again and investigate. My setup is currently being used for other purposes so might take a few days. -Anish
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c index ca11d28..7bc44e1 100644 --- a/net/dcb/dcbnl.c +++ b/net/dcb/dcbnl.c @@ -1914,6 +1914,8 @@ static int __init dcbnl_init(void) { INIT_LIST_HEAD(&dcb_app_list); + spin_lock_init(&dcb_lock); + rtnl_register(PF_UNSPEC, RTM_GETDCB, dcb_doit, NULL, NULL); rtnl_register(PF_UNSPEC, RTM_SETDCB, dcb_doit, NULL, NULL);
dcb_lock was being used uninitialized in dcbnl and is infact missing initialization code. Fixed Signed-off-by: Anish Bhatt <anish@chelsio.com> --- net/dcb/dcbnl.c | 2 ++ 1 file changed, 2 insertions(+)