[PATCH 2/2] Cleanup freeing of work struct

[Jan Kara]

Signed-off-by: Jan Kara <jack@xxxxxxx>
---
 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


--tThc/1wpZn/ma/RB--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/