vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
attributes triggering the lock-class key not persistent warning. Call
sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
vhci_hcd vhci_hcd: USB/IP Virtual Host Controller
vhci_hcd vhci_hcd: new USB bus registered, assigned bus number 2
BUG: key ffff88006a7e8d18 not in .data!
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
lockdep_init_map+0x60c/0x770
DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
[<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
[<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
[<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770 kernel/locking/lockdep.c:3131
[<ffffffff819e43d4>] __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
[<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520 fs/sysfs/file.c:305
[< inline >] create_files fs/sysfs/group.c:64
[<ffffffff819e8a89>] internal_create_group+0x239/0x8f0 fs/sysfs/group.c:134
[<ffffffff819e915f>] sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
[<ffffffff8323de24>] vhci_start+0x5b4/0x7a0 drivers/usb/usbip/vhci_hcd.c:978
[<ffffffff82c907ca>] usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
[<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
drivers/usb/usbip/vhci_hcd.c:1103
---
---
---[ end trace c33c7b202cf3aac8 ]---
Signed-off-by: Shuah Khan <[email protected]>
Reported-by: Andrey Konovalov <[email protected]>
---
drivers/usb/usbip/vhci_sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index c404017..b96e5b1 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -361,6 +361,7 @@ static void set_status_attr(int id)
status->attr.attr.name = status->name;
status->attr.attr.mode = S_IRUGO;
status->attr.show = status_show;
+ sysfs_attr_init(&status->attr.attr);
}
static int init_status_attrs(void)
--
2.7.4
Hi Andrey,
On 12/05/2016 12:56 PM, Shuah Khan wrote:
> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
> attributes triggering the lock-class key not persistent warning. Call
> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
>
> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller
> vhci_hcd vhci_hcd: new USB bus registered, assigned bus number 2
> BUG: key ffff88006a7e8d18 not in .data!
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
> lockdep_init_map+0x60c/0x770
> DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
> ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
> ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
> Call Trace:
> [< inline >] __dump_stack lib/dump_stack.c:15
> [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
> [<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770 kernel/locking/lockdep.c:3131
> [<ffffffff819e43d4>] __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520 fs/sysfs/file.c:305
> [< inline >] create_files fs/sysfs/group.c:64
> [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0 fs/sysfs/group.c:134
> [<ffffffff819e915f>] sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0 drivers/usb/usbip/vhci_hcd.c:978
> [<ffffffff82c907ca>] usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
> drivers/usb/usbip/vhci_hcd.c:1103
> ---
> ---
> ---[ end trace c33c7b202cf3aac8 ]---
>
> Signed-off-by: Shuah Khan <[email protected]>
> Reported-by: Andrey Konovalov <[email protected]>
Here is the fix. Fixed the warning I reproduced on my system.
Let me know if it works for you.
thanks,
-- Shuah
> ---
> drivers/usb/usbip/vhci_sysfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
> index c404017..b96e5b1 100644
> --- a/drivers/usb/usbip/vhci_sysfs.c
> +++ b/drivers/usb/usbip/vhci_sysfs.c
> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
> status->attr.attr.name = status->name;
> status->attr.attr.mode = S_IRUGO;
> status->attr.show = status_show;
> + sysfs_attr_init(&status->attr.attr);
> }
>
> static int init_status_attrs(void)
>
On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan <[email protected]> wrote:
> Hi Andrey,
>
> On 12/05/2016 12:56 PM, Shuah Khan wrote:
>> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
>> attributes triggering the lock-class key not persistent warning. Call
>> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
>>
>> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller
>> vhci_hcd vhci_hcd: new USB bus registered, assigned bus number 2
>> BUG: key ffff88006a7e8d18 not in .data!
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
>> lockdep_init_map+0x60c/0x770
>> DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
>> ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
>> ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
>> Call Trace:
>> [< inline >] __dump_stack lib/dump_stack.c:15
>> [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
>> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
>> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
>> [<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770 kernel/locking/lockdep.c:3131
>> [<ffffffff819e43d4>] __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
>> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520 fs/sysfs/file.c:305
>> [< inline >] create_files fs/sysfs/group.c:64
>> [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0 fs/sysfs/group.c:134
>> [<ffffffff819e915f>] sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
>> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0 drivers/usb/usbip/vhci_hcd.c:978
>> [<ffffffff82c907ca>] usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
>> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
>> drivers/usb/usbip/vhci_hcd.c:1103
>> ---
>> ---
>> ---[ end trace c33c7b202cf3aac8 ]---
>>
>> Signed-off-by: Shuah Khan <[email protected]>
>> Reported-by: Andrey Konovalov <[email protected]>
>
> Here is the fix. Fixed the warning I reproduced on my system.
> Let me know if it works for you.
Hi Shuah,
This fixes the warning I've been seeing.
Thanks!
>
> thanks,
> -- Shuah
>
>> ---
>> drivers/usb/usbip/vhci_sysfs.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
>> index c404017..b96e5b1 100644
>> --- a/drivers/usb/usbip/vhci_sysfs.c
>> +++ b/drivers/usb/usbip/vhci_sysfs.c
>> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
>> status->attr.attr.name = status->name;
>> status->attr.attr.mode = S_IRUGO;
>> status->attr.show = status_show;
>> + sysfs_attr_init(&status->attr.attr);
>> }
>>
>> static int init_status_attrs(void)
>>
>
Dear Shuah,
I'm afraid there's one thing I have concerned.
I think it's better to move sysfs_attr_init() before
status->attr.attr.name = status->name;
Best Regards,
nobuo.iwata
//
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Andrey Konovalov
> Sent: Tuesday, December 06, 2016 9:07 PM
> To: Shuah Khan
> Cc: Greg Kroah-Hartman; Valentina Manea; Shuah Khan;
> [email protected]; LKML
> Subject: Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init_map
>
> On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan <[email protected]> wrote:
> > Hi Andrey,
> >
> > On 12/05/2016 12:56 PM, Shuah Khan wrote:
> >> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
> >> attributes triggering the lock-class key not persistent warning. Call
> >> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
> >>
> >> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller vhci_hcd vhci_hcd:
> >> new USB bus registered, assigned bus number 2
> >> BUG: key ffff88006a7e8d18 not in .data!
> >> ------------[ cut here ]------------
> >> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
> >> lockdep_init_map+0x60c/0x770
> >> DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
> >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58 Hardware
> >> name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> >> ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
> >> ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
> >> ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
> >> Call Trace:
> >> [< inline >] __dump_stack lib/dump_stack.c:15
> >> [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
> >> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
> >> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
> >> [<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770
> >> kernel/locking/lockdep.c:3131 [<ffffffff819e43d4>]
> >> __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520
> fs/sysfs/file.c:305
> >> [< inline >] create_files fs/sysfs/group.c:64
> >> [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0
> >> fs/sysfs/group.c:134 [<ffffffff819e915f>]
> >> sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
> >> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0
> >> drivers/usb/usbip/vhci_hcd.c:978 [<ffffffff82c907ca>]
> >> usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
> >> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
> >> drivers/usb/usbip/vhci_hcd.c:1103
> >> ---
> >> ---
> >> ---[ end trace c33c7b202cf3aac8 ]---
> >>
> >> Signed-off-by: Shuah Khan <[email protected]>
> >> Reported-by: Andrey Konovalov <[email protected]>
> >
> > Here is the fix. Fixed the warning I reproduced on my system.
> > Let me know if it works for you.
>
> Hi Shuah,
>
> This fixes the warning I've been seeing.
>
> Thanks!
>
> >
> > thanks,
> > -- Shuah
> >
> >> ---
> >> drivers/usb/usbip/vhci_sysfs.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/usb/usbip/vhci_sysfs.c
> >> b/drivers/usb/usbip/vhci_sysfs.c index c404017..b96e5b1 100644
> >> --- a/drivers/usb/usbip/vhci_sysfs.c
> >> +++ b/drivers/usb/usbip/vhci_sysfs.c
> >> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
> >> status->attr.attr.name = status->name;
> >> status->attr.attr.mode = S_IRUGO;
> >> status->attr.show = status_show;
> >> + sysfs_attr_init(&status->attr.attr);
> >> }
> >>
> >> static int init_status_attrs(void)
> >>
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected] More majordomo info at
> http://vger.kernel.org/majordomo-info.html
Hi Nobuo.iwata,
I noticed that in many places sysfs_attr_init() is called
before populating the fields such as name etc. However, I
don't think the order matters.
sysfs_attr_init() doesn't depend on name or any other fields
being set:
#define sysfs_attr_init(attr) \
do { \
static struct lock_class_key __key; \
\
(attr)->key = &__key; \
} while (0)
Are you concerned about something else?
thanks,
-- Shuah
On 12/07/2016 07:15 PM, fx IWATA NOBUO wrote:
> Dear Shuah,
>
> I'm afraid there's one thing I have concerned.
>
> I think it's better to move sysfs_attr_init() before
> status->attr.attr.name = status->name;
>
> Best Regards,
>
> nobuo.iwata
> //
>> -----Original Message-----
>> From: [email protected]
>> [mailto:[email protected]] On Behalf Of Andrey Konovalov
>> Sent: Tuesday, December 06, 2016 9:07 PM
>> To: Shuah Khan
>> Cc: Greg Kroah-Hartman; Valentina Manea; Shuah Khan;
>> [email protected]; LKML
>> Subject: Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init_map
>>
>> On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan <[email protected]> wrote:
>>> Hi Andrey,
>>>
>>> On 12/05/2016 12:56 PM, Shuah Khan wrote:
>>>> vhci_hcd calls sysfs_create_group() with dynamically allocated sysfs
>>>> attributes triggering the lock-class key not persistent warning. Call
>>>> sysfs_attr_init() for dynamically allocated sysfs attributes to fix it.
>>>>
>>>> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller vhci_hcd vhci_hcd:
>>>> new USB bus registered, assigned bus number 2
>>>> BUG: key ffff88006a7e8d18 not in .data!
>>>> ------------[ cut here ]------------
>>>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
>>>> lockdep_init_map+0x60c/0x770
>>>> DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58 Hardware
>>>> name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>>> ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02 1ffff1000d79cd6a
>>>> ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3 ffffffff8598af40
>>>> ffffffff81f969f8 0000000000000000 0000000041b58ab3 0000000000000200
>>>> Call Trace:
>>>> [< inline >] __dump_stack lib/dump_stack.c:15
>>>> [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
>>>> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
>>>> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:565
>>>> [<ffffffff813f3efc>] lockdep_init_map+0x60c/0x770
>>>> kernel/locking/lockdep.c:3131 [<ffffffff819e43d4>]
>>>> __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
>> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520
>> fs/sysfs/file.c:305
>>>> [< inline >] create_files fs/sysfs/group.c:64
>>>> [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0
>>>> fs/sysfs/group.c:134 [<ffffffff819e915f>]
>>>> sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
>>>> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0
>>>> drivers/usb/usbip/vhci_hcd.c:978 [<ffffffff82c907ca>]
>>>> usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
>>>> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
>>>> drivers/usb/usbip/vhci_hcd.c:1103
>>>> ---
>>>> ---
>>>> ---[ end trace c33c7b202cf3aac8 ]---
>>>>
>>>> Signed-off-by: Shuah Khan <[email protected]>
>>>> Reported-by: Andrey Konovalov <[email protected]>
>>>
>>> Here is the fix. Fixed the warning I reproduced on my system.
>>> Let me know if it works for you.
>>
>> Hi Shuah,
>>
>> This fixes the warning I've been seeing.
>>
>> Thanks!
>>
>>>
>>> thanks,
>>> -- Shuah
>>>
>>>> ---
>>>> drivers/usb/usbip/vhci_sysfs.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/usb/usbip/vhci_sysfs.c
>>>> b/drivers/usb/usbip/vhci_sysfs.c index c404017..b96e5b1 100644
>>>> --- a/drivers/usb/usbip/vhci_sysfs.c
>>>> +++ b/drivers/usb/usbip/vhci_sysfs.c
>>>> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
>>>> status->attr.attr.name = status->name;
>>>> status->attr.attr.mode = S_IRUGO;
>>>> status->attr.show = status_show;
>>>> + sysfs_attr_init(&status->attr.attr);
>>>> }
>>>>
>>>> static int init_status_attrs(void)
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to [email protected] More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
Dear Shuah,
> I noticed that in many places sysfs_attr_init() is called before populating
> the fields such as name etc. However, I don't think the order matters.
>
> sysfs_attr_init() doesn't depend on name or any other fields being set:
>
> #define sysfs_attr_init(attr) \
> do { \
> static struct lock_class_key __key; \
> \
> (attr)->key = &__key; \
> } while (0)
>
> Are you concerned about something else?
Yes.
If the macro name is 'attr_set_key', I don't have any concern.
The name is 'attr_init' so I think there's possibility that some other
items in attr may initialized in future.
Also I think 'initialize struct first, then set items' is idiomatic order.
Best Regards,
nobuo.iwata
//
> -----Original Message-----
> From: Shuah Khan [mailto:[email protected]]
> Sent: Thursday, December 08, 2016 11:51 AM
> To: fx IWATA NOBUO; Andrey Konovalov
> Cc: Greg Kroah-Hartman; Valentina Manea; Shuah Khan;
> [email protected]; LKML; Shuah Khan; Shuah Khan
> Subject: Re: [PATCH] usbip: fix warning in vhci_hcd_probe/lockdep_init_map
>
> Hi Nobuo.iwata,
>
> I noticed that in many places sysfs_attr_init() is called before populating
> the fields such as name etc. However, I don't think the order matters.
>
> sysfs_attr_init() doesn't depend on name or any other fields being set:
>
> #define sysfs_attr_init(attr) \
> do { \
> static struct lock_class_key __key; \
> \
> (attr)->key = &__key; \
> } while (0)
>
> Are you concerned about something else?
>
> thanks,
> -- Shuah
>
>
> On 12/07/2016 07:15 PM, fx IWATA NOBUO wrote:
> > Dear Shuah,
> >
> > I'm afraid there's one thing I have concerned.
> >
> > I think it's better to move sysfs_attr_init() before
> > status->attr.attr.name = status->name;
> >
> > Best Regards,
> >
> > nobuo.iwata
> > //
> >> -----Original Message-----
> >> From: [email protected]
> >> [mailto:[email protected]] On Behalf Of Andrey
> >> Konovalov
> >> Sent: Tuesday, December 06, 2016 9:07 PM
> >> To: Shuah Khan
> >> Cc: Greg Kroah-Hartman; Valentina Manea; Shuah Khan;
> >> [email protected]; LKML
> >> Subject: Re: [PATCH] usbip: fix warning in
> >> vhci_hcd_probe/lockdep_init_map
> >>
> >> On Mon, Dec 5, 2016 at 9:00 PM, Shuah Khan <[email protected]>
> wrote:
> >>> Hi Andrey,
> >>>
> >>> On 12/05/2016 12:56 PM, Shuah Khan wrote:
> >>>> vhci_hcd calls sysfs_create_group() with dynamically allocated
> >>>> sysfs attributes triggering the lock-class key not persistent
> >>>> warning. Call
> >>>> sysfs_attr_init() for dynamically allocated sysfs attributes to fix
> it.
> >>>>
> >>>> vhci_hcd vhci_hcd: USB/IP Virtual Host Controller vhci_hcd vhci_hcd:
> >>>> new USB bus registered, assigned bus number 2
> >>>> BUG: key ffff88006a7e8d18 not in .data!
> >>>> ------------[ cut here ]------------
> >>>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3131
> >>>> lockdep_init_map+0x60c/0x770
> >>>> DEBUG_LOCKS_WARN_ON(1)[ 1.567044] Modules linked in:
> >>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc7+ #58 Hardware
> >>>> name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> >>>> ffff88006bce6eb8 ffffffff81f96c8a ffffffff00000a02
> >>>> 1ffff1000d79cd6a
> >>>> ffffed000d79cd62 000000046bce6ed8 0000000041b58ab3
> >>>> ffffffff8598af40
> >>>> ffffffff81f969f8 0000000000000000 0000000041b58ab3
> >>>> 0000000000000200 Call Trace:
> >>>> [< inline >] __dump_stack lib/dump_stack.c:15
> >>>> [<ffffffff81f96c8a>] dump_stack+0x292/0x398 lib/dump_stack.c:51
> >>>> [<ffffffff812b808f>] __warn+0x19f/0x1e0 kernel/panic.c:550
> >>>> [<ffffffff812b8195>] warn_slowpath_fmt+0xc5/0x110
> >>>> kernel/panic.c:565 [<ffffffff813f3efc>]
> >>>> lockdep_init_map+0x60c/0x770
> >>>> kernel/locking/lockdep.c:3131 [<ffffffff819e43d4>]
> >>>> __kernfs_create_file+0x114/0x2a0 fs/kernfs/file.c:954
> >> [<ffffffff819e68f5>] sysfs_add_file_mode_ns+0x225/0x520
> >> fs/sysfs/file.c:305
> >>>> [< inline >] create_files fs/sysfs/group.c:64
> >>>> [<ffffffff819e8a89>] internal_create_group+0x239/0x8f0
> >>>> fs/sysfs/group.c:134 [<ffffffff819e915f>]
> >>>> sysfs_create_group+0x1f/0x30 fs/sysfs/group.c:156
> >>>> [<ffffffff8323de24>] vhci_start+0x5b4/0x7a0
> >>>> drivers/usb/usbip/vhci_hcd.c:978 [<ffffffff82c907ca>]
> >>>> usb_add_hcd+0x8da/0x1c60 drivers/usb/core/hcd.c:2867
> >>>> [<ffffffff8323bc57>] vhci_hcd_probe+0x97/0x130
> >>>> drivers/usb/usbip/vhci_hcd.c:1103
> >>>> ---
> >>>> ---
> >>>> ---[ end trace c33c7b202cf3aac8 ]---
> >>>>
> >>>> Signed-off-by: Shuah Khan <[email protected]>
> >>>> Reported-by: Andrey Konovalov <[email protected]>
> >>>
> >>> Here is the fix. Fixed the warning I reproduced on my system.
> >>> Let me know if it works for you.
> >>
> >> Hi Shuah,
> >>
> >> This fixes the warning I've been seeing.
> >>
> >> Thanks!
> >>
> >>>
> >>> thanks,
> >>> -- Shuah
> >>>
> >>>> ---
> >>>> drivers/usb/usbip/vhci_sysfs.c | 1 +
> >>>> 1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/drivers/usb/usbip/vhci_sysfs.c
> >>>> b/drivers/usb/usbip/vhci_sysfs.c index c404017..b96e5b1 100644
> >>>> --- a/drivers/usb/usbip/vhci_sysfs.c
> >>>> +++ b/drivers/usb/usbip/vhci_sysfs.c
> >>>> @@ -361,6 +361,7 @@ static void set_status_attr(int id)
> >>>> status->attr.attr.name = status->name;
> >>>> status->attr.attr.mode = S_IRUGO;
> >>>> status->attr.show = status_show;
> >>>> + sysfs_attr_init(&status->attr.attr);
> >>>> }
> >>>>
> >>>> static int init_status_attrs(void)
> >>>>
> >>>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-usb"
> >> in the body of a message to [email protected] More majordomo
> >> info at http://vger.kernel.org/majordomo-info.html
On 12/07/2016 09:12 PM, fx IWATA NOBUO wrote:
> Dear Shuah,
>
>> I noticed that in many places sysfs_attr_init() is called before populating
>> the fields such as name etc. However, I don't think the order matters.
>>
>> sysfs_attr_init() doesn't depend on name or any other fields being set:
>>
>> #define sysfs_attr_init(attr) \
>> do { \
>> static struct lock_class_key __key; \
>> \
>> (attr)->key = &__key; \
>> } while (0)
>>
>> Are you concerned about something else?
>
> Yes.
>
> If the macro name is 'attr_set_key', I don't have any concern.
>
> The name is 'attr_init' so I think there's possibility that some other
> items in attr may initialized in future.
>
> Also I think 'initialize struct first, then set items' is idiomatic order.
>
> Best Regards,
>
I am seeing places where sysfs_attr_init() after setting the name as well.
As this is more of a better done this way - I will fix this in the next
release. It is more important to fix the lockdep warning.
thanks,
-- Shuah