>From 54366b186d1e50b3453dfd2881a7eae559ab6cbc Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Thu, 6 Aug 2009 22:24:57 +0200 Subject: [PATCH 2/2] Cleanup freeing of work struct Signed-off-by: Jan Kara --- fs/fs-writeback.c | 121 ++++++++++++++++++++++++++++++---------------------- 1 files changed, 70 insertions(+), 51 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index fc7e4a3..bdc8dd5 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -61,16 +61,35 @@ struct bdi_work { }; enum { - WS_USED_B = 0, - WS_ONSTACK_B, + WS_USED_B = 0, /* Still processed by the writeback thread */ + WS_ALLOCATED_B, /* Work is dynamically allocated */ + WS_ASYNC_B, /* Should be freed by the writeback thread */ }; #define WS_USED (1 << WS_USED_B) -#define WS_ONSTACK (1 << WS_ONSTACK_B) +#define WS_ALLOCATED (1 << WS_ALLOCATED_B) +#define WS_ASYNC (1 << WS_ASYNC_B) -static inline bool bdi_work_on_stack(struct bdi_work *work) +static inline bool bdi_work_async(struct bdi_work *work) { - return test_bit(WS_ONSTACK_B, &work->state); + + return test_bit(WS_ASYNC_B, &work->state); +} + +static inline bool bdi_work_allocated(struct bdi_work *work) +{ + return test_bit(WS_ALLOCATED_B, &work->state); +} + +static inline void bdi_set_work_async(struct bdi_work *work) +{ + WARN_ON(!bdi_work_allocated(work)); + set_bit(WS_ASYNC_B, &work->state); +} + +static inline void bdi_set_work_allocated(struct bdi_work *work) +{ + set_bit(WS_ALLOCATED_B, &work->state); } static inline void bdi_work_init(struct bdi_work *work, struct super_block *sb, @@ -84,15 +103,6 @@ static inline void bdi_work_init(struct bdi_work *work, struct super_block *sb, work->state = WS_USED; } -static inline void bdi_work_init_on_stack(struct bdi_work *work, - struct super_block *sb, - unsigned long nr_pages, - enum writeback_sync_modes sync_mode) -{ - bdi_work_init(work, sb, nr_pages, sync_mode); - work->state |= WS_ONSTACK; -} - /** * writeback_in_progress - determine whether there is writeback in progress * @bdi: the device's backing_dev_info structure. @@ -116,26 +126,19 @@ 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); + kfree(work); } static void wb_work_complete(struct bdi_work *work) { - const enum writeback_sync_modes sync_mode = work->sync_mode; - /* - * 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. + * Either free the work if it is ASYNC one (and thus nobody + * should wait on it) or mark it as done. */ - if (!bdi_work_on_stack(work)) - bdi_work_clear(work); - if (sync_mode == WB_SYNC_NONE || bdi_work_on_stack(work)) + if (bdi_work_async(work)) call_rcu(&work->rcu_head, bdi_work_free); + else + bdi_work_clear(work); } static void wb_clear_pending(struct bdi_writeback *wb, struct bdi_work *work) @@ -213,8 +216,9 @@ static void bdi_queue_work(struct backing_dev_info *bdi, struct bdi_work *work) } /* - * 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. + * Wait until the writeback thread is done with the work. For WB_SYNC_NONE + * work, it is the moment the thread has copied the contents of the structure, + * for WB_SYNC_ALL work, it is the moment the thread has submitted all the IO. */ static void bdi_wait_on_work_clear(struct bdi_work *work) { @@ -228,44 +232,58 @@ static struct bdi_work *bdi_alloc_work(struct super_block *sb, long nr_pages, struct bdi_work *work; work = kmalloc(sizeof(*work), GFP_ATOMIC); - if (work) + if (work) { bdi_work_init(work, sb, nr_pages, sync_mode); + bdi_set_work_allocated(work); + } return work; } +/* + * Wait until the work is safe to be freed and free it. + * + * Note that for WB_SYNC_ALL work, this waits until all the IO is submitted + */ +static void bdi_wait_free_work(struct bdi_work *work) +{ + if (!bdi_work_async(work)) { + bdi_wait_on_work_clear(work); + if (bdi_work_allocated(work)) + call_rcu(&work->rcu_head, bdi_work_free); + else { + /* + * For on-stack work, we have to wait as the structure + * on stack can be freed as soon as we return. + */ + synchronize_rcu(); + } + } +} + void bdi_start_writeback(struct backing_dev_info *bdi, struct super_block *sb, long nr_pages, enum writeback_sync_modes sync_mode) { const bool must_wait = sync_mode == WB_SYNC_ALL; struct bdi_work work_stack, *work = NULL; - if (!must_wait) + if (!must_wait) { work = bdi_alloc_work(sb, nr_pages, sync_mode); + /* + * Mark that we don't want to wait on work and the writeback + * thread can free it. + */ + if (work) + bdi_set_work_async(work); + } if (!work) { work = &work_stack; - bdi_work_init_on_stack(work, sb, nr_pages, sync_mode); + bdi_work_init(work, sb, nr_pages, sync_mode); } bdi_queue_work(bdi, work); - - /* - * If the sync mode is WB_SYNC_ALL, block waiting for the work to - * complete. If not, we only need to wait for the work to be started, - * if we allocated it on-stack. We use the same mechanism, if the - * wait bit is set in the bdi_work struct, then threads will not - * clear pending until after they are done. - * - * Note that work == &work_stack if must_wait is true, so we don't - * need to do call_rcu() here ever, since the completion path will - * have done that for us. - */ - if (must_wait || work == &work_stack) { - bdi_wait_on_work_clear(work); - if (work != &work_stack) - call_rcu(&work->rcu_head, bdi_work_free); - } + bdi_wait_free_work(work); } /* @@ -507,6 +525,8 @@ restart: } if (must_wait) list_add_tail(&work->wait_list, &list); + else + bdi_set_work_async(work); bdi_queue_work(bdi, work); } @@ -520,8 +540,7 @@ restart: while (!list_empty(&list)) { work = list_entry(list.next, struct bdi_work, wait_list); list_del(&work->wait_list); - bdi_wait_on_work_clear(work); - call_rcu(&work->rcu_head, bdi_work_free); + bdi_wait_free_work(work); } } -- 1.6.0.2