Messages in this thread | | | Date | Mon, 23 Oct 2017 07:03:21 -0700 | From | Tejun Heo <> | Subject | Re: [Question] null pointer risk of kernel workqueue |
| |
Hello,
On Mon, Oct 23, 2017 at 09:34:11AM +0800, Li Bin wrote: > > > on 2017/10/21 23:35, Tejun Heo wrote: > > On Fri, Oct 20, 2017 at 02:57:18PM +0800, tanxiaofei wrote: > >> Hi Tejun, > >> > >> Any comments about this? > > > > I think you're confused, or at least can't understand what you're > > trying to say. Can you create a rero? > > > > Hi Tejun, > The case is as following: > > worker_thread() > |-spin_lock_irq() > |-process_one_work() > |-worker->current_pwq = pwq > |-spin_unlock_irq() > |-worker->current_func(work) > |-spin_lock_irq() > |-worker->current_pwq = NULL > |-spin_unlock_irq() > //interrupt here > |-irq_handler > |-__queue_work() > //assuming that the wq is draining > |-if (unlikely(wq->flags & __WQ_DRAINING) &&WARN_ON_ONCE(!is_chained_work(wq))) > |-is_chained_work(wq) > |-current_wq_worker() // Here, 'current' is the interrupted worker! > |-current->current_pwq is NULL here! > |-schedule() > > And I think the following patch can solve the bug, right? > > diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h > index 8635417..650680c 100644 > --- a/kernel/workqueue_internal.h > +++ b/kernel/workqueue_internal.h > @@ -59,7 +59,7 @@ struct worker { > */ > static inline struct worker *current_wq_worker(void) > { > - if (current->flags & PF_WQ_WORKER) > + if (!in_irq() && (current->flags & PF_WQ_WORKER)) > return kthread_data(current); > return NULL; > }
Yeah, that makes sense to me. Can you please resend the patch with patch description and SOB?
Thanks.
-- tejun
| |