The code dealing with bdi_work->state and completion of a bdi_work is a major mess currently. This patch makes sure we directly use one set of flags to deal with it, and use it consistently, which means: - always notify about completion from the rcu callback. We only ever wait for it from on-stack callers, so this simplification does not even cause a theoretical slowdown currently. It also makes sure we don't miss out on the notification if we ever add other callers to wait for it. - make earlier completion notification depending on the on-stack allocation, not the sync mode. If we introduce new callers that want to do WB_SYNC_NONE writeback from on-stack callers this will be nessecary. Also rename bdi_wait_on_work_clear to bdi_wait_on_work_done and inline a few small functions into their only caller to make the code understandable. Signed-off-by: Christoph Hellwig <hch@xxxxxx> Index: linux-2.6/fs/fs-writeback.c =================================================================== --- linux-2.6.orig/fs/fs-writeback.c 2010-06-08 16:17:45.781254323 +0200 +++ linux-2.6/fs/fs-writeback.c 2010-06-08 16:20:11.916005756 +0200 @@ -63,24 +63,16 @@ struct bdi_work { }; enum { - WS_USED_B = 0, - WS_ONSTACK_B, + WS_INPROGRESS = 0, + WS_ONSTACK, }; -#define WS_USED (1 << WS_USED_B) -#define WS_ONSTACK (1 << WS_ONSTACK_B) - -static inline bool bdi_work_on_stack(struct bdi_work *work) -{ - return test_bit(WS_ONSTACK_B, &work->state); -} - static inline void bdi_work_init(struct bdi_work *work, struct wb_writeback_args *args) { INIT_RCU_HEAD(&work->rcu_head); work->args = *args; - work->state = WS_USED; + __set_bit(WS_INPROGRESS, &work->state); } /** @@ -95,43 +87,16 @@ int writeback_in_progress(struct backing return !list_empty(&bdi->work_list); } -static void bdi_work_clear(struct bdi_work *work) -{ - clear_bit(WS_USED_B, &work->state); - smp_mb__after_clear_bit(); - /* - * work can have disappeared at this point. bit waitq functions - * should be able to tolerate this, provided bdi_sched_wait does - * not dereference it's pointer argument. - */ - wake_up_bit(&work->state, WS_USED_B); -} - static void bdi_work_free(struct rcu_head *head) { struct bdi_work *work = container_of(head, struct bdi_work, rcu_head); - if (!bdi_work_on_stack(work)) - kfree(work); - else - bdi_work_clear(work); -} - -static void wb_work_complete(struct bdi_work *work) -{ - const enum writeback_sync_modes sync_mode = work->args.sync_mode; - int onstack = bdi_work_on_stack(work); + clear_bit(WS_INPROGRESS, &work->state); + smp_mb__after_clear_bit(); + wake_up_bit(&work->state, WS_INPROGRESS); - /* - * For allocated work, we can clear the done/seen bit right here. - * For on-stack work, we need to postpone both the clear and free - * to after the RCU grace period, since the stack could be invalidated - * as soon as bdi_work_clear() has done the wakeup. - */ - if (!onstack) - bdi_work_clear(work); - if (sync_mode == WB_SYNC_NONE || onstack) - call_rcu(&work->rcu_head, bdi_work_free); + if (!test_bit(WS_ONSTACK, &work->state)) + kfree(work); } static void wb_clear_pending(struct bdi_writeback *wb, struct bdi_work *work) @@ -147,7 +112,7 @@ static void wb_clear_pending(struct bdi_ list_del_rcu(&work->list); spin_unlock(&bdi->wb_lock); - wb_work_complete(work); + call_rcu(&work->rcu_head, bdi_work_free); } } @@ -185,9 +150,9 @@ static void bdi_queue_work(struct backin * Used for on-stack allocated work items. The caller needs to wait until * the wb threads have acked the work before it's safe to continue. */ -static void bdi_wait_on_work_clear(struct bdi_work *work) +static void bdi_wait_on_work_done(struct bdi_work *work) { - wait_on_bit(&work->state, WS_USED_B, bdi_sched_wait, + wait_on_bit(&work->state, WS_INPROGRESS, bdi_sched_wait, TASK_UNINTERRUPTIBLE); } @@ -234,10 +199,10 @@ static void bdi_sync_writeback(struct ba struct bdi_work work; bdi_work_init(&work, &args); - work.state |= WS_ONSTACK; + __set_bit(WS_ONSTACK, &work.state); bdi_queue_work(bdi, &work); - bdi_wait_on_work_clear(&work); + bdi_wait_on_work_done(&work); } /** @@ -911,7 +876,7 @@ long wb_do_writeback(struct bdi_writebac * If this isn't a data integrity operation, just notify * that we have seen this work and we are now starting it. */ - if (args.sync_mode == WB_SYNC_NONE) + if (!test_bit(WS_ONSTACK, &work->state)) wb_clear_pending(wb, work); wrote += wb_writeback(wb, &args); @@ -920,7 +885,7 @@ long wb_do_writeback(struct bdi_writebac * This is a data integrity writeback, so only do the * notification when we have completed the work. */ - if (args.sync_mode == WB_SYNC_ALL) + if (test_bit(WS_ONSTACK, &work->state)) wb_clear_pending(wb, work); } -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html