Re: + softirq-fix-tasklet_kill-and-its-users.patch added to -mm tree

[Santosh Shilimkar <santosh.shilimkar@oracle.com>]

On 9/21/2016 5:42 PM, Sergey Senozhatsky wrote:
> Hello,
>
> On (09/21/16 10:23), Santosh Shilimkar wrote:

[...]

>> Am assuming one of the driver in your test is using the DECLARE_TASKLET
>> to init the tasklet and killed by tasklet_kill() which leaves that
>> tasklet to be still scheduled by tasklet action.
>
> yes, vt does something like this (kbd_bh).
>
>> Can you please try below patch and see if you still see the issue ?
>> Attaching the same, just in case mailer eat the tabs.
>
> hm, didn't completely fix it. the vt is now happy, unlike usbnet.
Good that vt works now.

> and the usbnet case is rather alarming.
>
>
> looking at usbnet_probe()
>
> int
> usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> {
> ....
> 	skb_queue_head_init (&dev->done);
> 	skb_queue_head_init(&dev->rxq_pause);
> 	dev->bh.func = usbnet_bh;
> 	dev->bh.data = (unsigned long) dev;
> 	INIT_WORK (&dev->kevent, usbnet_deferred_kevent);
> ....
>
>
> first, sometimes tasklet initialisation is performed directly, not via
> tasklet_init().
>
> second, that 't->count == 0' eq 'tasklet_init()' is assumed to be sort of
> a contract. so a simple kzalloc() works fine, and the patch breaks it.
>
Thats really bad that tasklet code is letting users call 
tasklet_schedule() even without any tasklet_init or DECLARE_TASKLET.

> a simple grep in drivers/net/
>
> _next$ git grep tasklet_sched drivers/net/ | awk '{print $1}' | uniq | wc -l
> 60
>
> _next$ git grep tasklet_init drivers/net/ | awk '{print $1}' | uniq | wc -l
> 52
>
> and I don't know how many call-sites outside of drivers/net/ do something
> like this.
>
There are more :-(. Thanks for helping out Sergey.

# git grep tasklet_sched . | awk '{print $1}' | uniq | wc -l
269
# git grep tasklet_init . | awk '{print $1}' | uniq | wc -l
240

Andrew,
I requested you to include this patch but now am not sure anymore.
Looks like there are almost 30 more users which are directly
tweaking 'tasklet_struct' fields and calling other APIs. Hunting them
and fixing them probably would be an exercise and also those changes
needs those changed drivers to be tested.

What do you suggest ? At least this patch needs to be dropped as of now
till we can have complete coverage for those bad users.

Regards,
Santosh