lkml.org 
[lkml]   [2010]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [regression] Crash in wb_clear_pending()
    On 05/07/10 21.32, Christoph Hellwig wrote:
    > On Mon, Jul 05, 2010 at 09:24:39PM +0200, Jens Axboe wrote:
    >> The oops itself looks like a recurrence of the missing RCU grace or
    >> too early stack wakeup, which should be a 1-2 liner once it's found.
    >
    > See the previous thread. There's at least two issues:
    >
    > - wb_do_writeback checks work->state after it's been freed when we do
    > the second test_bit for WS_ONSTACK
    > - bdi_work_free accesses work->state after waking up the caller doing
    > bdi_wait_on_work_done, which might have re-used the stack space
    > allocated for the work item.
    >
    > The fix for that is to get rid of the fragile work->state stuff and the
    > bit wakeups by just using a completion and using that as indicator
    > for the stack wait. That's the main change the above patch does. In
    > addition it also merges the two structures used for the writeback
    > requests. Onl doing the completion and earlier list removal would
    > be something like the untested patch below:

    If those two late ON_STACK checks is the only issue left there,
    why not just apply the below for 2.6.35?

    diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
    index 0609607..15ce6ab 100644
    --- a/fs/fs-writeback.c
    +++ b/fs/fs-writeback.c
    @@ -90,12 +90,13 @@ int writeback_in_progress(struct backing_dev_info *bdi)
    static void bdi_work_free(struct rcu_head *head)
    {
    struct bdi_work *work = container_of(head, struct bdi_work, rcu_head);
    + int on_stack = test_bit(WS_ONSTACK, &work->state);

    clear_bit(WS_INPROGRESS, &work->state);
    smp_mb__after_clear_bit();
    wake_up_bit(&work->state, WS_INPROGRESS);

    - if (!test_bit(WS_ONSTACK, &work->state))
    + if (!on_stack)
    kfree(work);
    }

    @@ -854,6 +855,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait)

    while ((work = get_next_work_item(bdi, wb)) != NULL) {
    struct wb_writeback_args args = work->args;
    + int on_stack = test_bit(WS_ONSTACK, &work->state);

    /*
    * Override sync mode, in case we must wait for completion
    @@ -865,7 +867,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
    * If this isn't a data integrity operation, just notify
    * that we have seen this work and we are now starting it.
    */
    - if (!test_bit(WS_ONSTACK, &work->state))
    + if (!on_stack)
    wb_clear_pending(wb, work);

    wrote += wb_writeback(wb, &args);
    @@ -874,7 +876,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait)
    * This is a data integrity writeback, so only do the
    * notification when we have completed the work.
    */
    - if (test_bit(WS_ONSTACK, &work->state))
    + if (on_stack)
    wb_clear_pending(wb, work);
    }

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2010-07-05 21:49    [W:0.047 / U:0.076 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site