Hi Julian, On 10.03.2016 01:42, Julian Calaby wrote: > Hi Vladimir, > > On Thu, Mar 10, 2016 at 10:30 AM, Vladimir Zapolskiy <v...@mleia.com> wrote: >> Hi Julian, >> >> On 10.03.2016 01:27, Julian Calaby wrote: >>> Hi Vladimir, >>> >>> On Thu, Mar 10, 2016 at 10:13 AM, Vladimir Zapolskiy <v...@mleia.com> wrote: >>>> The kthread_run() function returns either a valid task_struct or >>>> ERR_PTR() value, check for NULL is invalid. The change fixes potential >>>> oops, e.g. in OOM situation. >>>> >>>> Signed-off-by: Vladimir Zapolskiy <v...@mleia.com> >>>> --- >>>> drivers/staging/wilc1000/linux_wlan.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/staging/wilc1000/linux_wlan.c >>>> b/drivers/staging/wilc1000/linux_wlan.c >>>> index 54fe9d7..5077c30 100644 >>>> --- a/drivers/staging/wilc1000/linux_wlan.c >>>> +++ b/drivers/staging/wilc1000/linux_wlan.c >>>> @@ -849,10 +849,10 @@ static int wlan_initialize_threads(struct net_device >>>> *dev) >>>> PRINT_D(INIT_DBG, "Creating kthread for transmission\n"); >>>> wilc->txq_thread = kthread_run(linux_wlan_txq_task, (void *)dev, >>>> "K_TXQ_TASK"); >>>> - if (!wilc->txq_thread) { >>>> + if (IS_ERR(wilc->txq_thread)) { >>>> PRINT_ER("couldn't create TXQ thread\n"); >>>> wilc->close = 0; >>>> - return -ENOBUFS; >>>> + return PTR_ERR(wilc->txq_thread); >>> >>> Are you sure changing the error returned is correct? Do all the >>> callers of wlan_initialize_threads() handle the full range of errors >>> from kthread_run()? >> >> Have you checked the driver? > > I'm making sure you have. It's possible that there's a good reason why > this returns -ENOBUFS I want to know that you've at least considered > that possibility.
You have my confirmation, I've checked the call stack before publishing this fix. >> This function is called once on initialization, the check on the upper layer >> has "if (ret < 0) goto exit_badly;" form. > > And practically everything in the chain up to net_device_ops uses the > same error handling scheme so it's probably fine. dev_open() __dev_open() wilc_mac_open() wilc1000_wlan_init() wlan_initialize_threads() Oh, why kernel threads within a driver are init'ed/destroyed on each device up/down state transition? > You should also document this change in the commit message. The change is documented in the commit message, take a look. But I didn't add "I swear it does not break anything" ;) -- With best wishes, Vladimir _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel