lkml.org 
[lkml]   [2010]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 14/16] writeback: move bdi threads exiting logic to the forker thread
    Yes, only killing threads from the caller is much better, that's how
    the kthread API is supposed to be used anyway.

    > static void bdi_queue_work(struct backing_dev_info *bdi,
    > struct wb_writeback_work *work)
    > {
    > + bool wakeup_default = false;
    > +
    > trace_writeback_queue(bdi, work);
    >
    > spin_lock(&bdi->wb_lock);
    > list_add_tail(&work->list, &bdi->work_list);
    > - spin_unlock(&bdi->wb_lock);
    > -
    > /*
    > * If the default thread isn't there, make sure we add it. When
    > * it gets created and wakes up, we'll run this work.
    > */
    > - if (unlikely(!bdi->wb.task)) {
    > + if (unlikely(!bdi->wb.task))
    > + wakeup_default = true;
    > + else
    > + wake_up_process(bdi->wb.task);
    > + spin_unlock(&bdi->wb_lock);
    > +
    > + if (wakeup_default) {
    > trace_writeback_nothread(bdi, work);
    > wake_up_process(default_backing_dev_info.wb.task);

    Why not simply do the defaul thread wakeup under wb_lock, too?
    It keeps the code a lot simpler, and this is not a typical path anyway.

    > if (dirty_writeback_interval) {
    > + unsigned long wait_jiffies;
    > +
    > wait_jiffies = msecs_to_jiffies(dirty_writeback_interval * 10);
    > schedule_timeout(wait_jiffies);

    No real need for a local variable here.

    > @@ -364,7 +395,7 @@ static int bdi_forker_thread(void *ptr)
    > if (!list_empty(&me->bdi->work_list))
    > __set_current_state(TASK_RUNNING);
    >
    > - if (!fork) {
    > + if (!fork && !kill) {

    I think the code here would be a lot cleaner if you implement the
    suggestion I have for the forking restructuring.



    \
     
     \ /
      Last update: 2010-07-18 09:05    [W:0.031 / U:0.072 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site