From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754942AbcIVCco (ORCPT ); Wed, 21 Sep 2016 22:32:44 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:46351 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752607AbcIVCcm (ORCPT ); Wed, 21 Sep 2016 22:32:42 -0400 Subject: Re: + softirq-fix-tasklet_kill-and-its-users.patch added to -mm tree To: Sergey Senozhatsky , akpm@linux-foundation.org References: <57e1b041.zRoBcsxStpPQoyeo%akpm@linux-foundation.org> <20160921051810.GA396@swordfish> <20160921080942.GA476@swordfish> <20160922004204.GA701@swordfish> Cc: ssantosh@kernel.org, davem@davemloft.net, giovanni.cabiddu@intel.com, gregkh@linuxfoundation.org, herbert@gondor.apana.org.au, isdn@linux-pingi.de, mingo@elte.hu, pebolle@tiscali.nl, peterz@infradead.org, salvatore.benedetto@intel.com, tadeusz.struk@intel.com, tglx@linutronix.de, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org, sfr@canb.auug.org.au, linux-next@vger.kernel.org, sergey.senozhatsky@gmail.com From: Santosh Shilimkar Organization: Oracle Corporation Message-ID: <36fffd08-17ec-0a73-17f3-378597e0c25a@oracle.com> Date: Wed, 21 Sep 2016 19:31:30 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160922004204.GA701@swordfish> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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