workqueue: reorganize try_to_grab_pending() and __cancel_timer_work()

* Use bool @is_dwork instead of @timer and let try_to_grab_pending() use to_delayed_work() to determine the delayed_work address. * Move timer handling from __cancel_work_timer() to try_to_grab_pending(). * Make try_to_grab_pending() use -EAGAIN instead of -1 for busy-looping and drop the ret local variable. * Add proper function comment to try_to_grab_pending(). This makes the code a bit easier to understand and will ease further changes. This patch doesn't make any functional change. v2: Use @is_dwork instead of @timer. Signed-off-by: Tejun Heo <tj@kernel.org>

workqueue: reorganize try_to_grab_pending() and __cancel_timer_work()
* Use bool @is_dwork instead of @timer and let try_to_grab_pending() use to_delayed_work() to determine the delayed_work address. * Move timer handling from __cancel_work_timer() to try_to_grab_pending(). * Make try_to_grab_pending() use -EAGAIN instead of -1 for busy-looping and drop the ret local variable. * Add proper function comment to try_to_grab_pending(). This makes the code a bit easier to understand and will ease further changes. This patch doesn't make any functional change. v2: Use @is_dwork instead of @timer. Signed-off-by: Tejun Heo <tj@kernel.org>
Tejun Heo
1 parent 7beb2edf44
Showing 1 changed file with 32 additions and 15 deletions Side-by-side Diff
kernel/workqueue.c
@@ -1004,15 +1004,33 @@
 		complete(&cwq->wq->first_flusher->done);
 }
  
-/*
- * Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit,
- * so this work can't be re-armed in any way.
+/**
+ * try_to_grab_pending - steal work item from worklist
+ * @work: work item to steal
+ * @is_dwork: @work is a delayed_work
+ *
+ * Try to grab PENDING bit of @work.  This function can handle @work in any
+ * stable state - idle, on timer or on worklist.  Return values are
+ *
+ *  1		if @work was pending and we successfully stole PENDING
+ *  0		if @work was idle and we claimed PENDING
+ *  -EAGAIN	if PENDING couldn't be grabbed at the moment, safe to busy-retry
+ *
+ * On >= 0 return, the caller owns @work's PENDING bit.
  */
-static int try_to_grab_pending(struct work_struct *work)
+static int try_to_grab_pending(struct work_struct *work, bool is_dwork)
 {
 	struct global_cwq *gcwq;
-	int ret = -1;
  
+	/* try to steal the timer if it exists */
+	if (is_dwork) {
+		struct delayed_work *dwork = to_delayed_work(work);
+
+		if (likely(del_timer(&dwork->timer)))
+			return 1;
+	}
+
+	/* try to claim PENDING the normal way */
 	if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)))
 		return 0;
  
@@ -1022,7 +1040,7 @@
 	 */
 	gcwq = get_work_gcwq(work);
 	if (!gcwq)
-		return ret;
+		return -EAGAIN;
  
 	spin_lock_irq(&gcwq->lock);
 	if (!list_empty(&work->entry)) {
  
@@ -1038,12 +1056,14 @@
 			cwq_dec_nr_in_flight(get_work_cwq(work),
 				get_work_color(work),
 				*work_data_bits(work) & WORK_STRUCT_DELAYED);
-			ret = 1;
+
+			spin_unlock_irq(&gcwq->lock);
+			return 1;
 		}
 	}
 	spin_unlock_irq(&gcwq->lock);
  
-	return ret;
+	return -EAGAIN;
 }
  
 /**
  
@@ -2817,15 +2837,12 @@
 }
 EXPORT_SYMBOL_GPL(flush_work_sync);
  
-static bool __cancel_work_timer(struct work_struct *work,
-				struct timer_list* timer)
+static bool __cancel_work_timer(struct work_struct *work, bool is_dwork)
 {
 	int ret;
  
 	do {
-		ret = (timer && likely(del_timer(timer)));
-		if (!ret)
-			ret = try_to_grab_pending(work);
+		ret = try_to_grab_pending(work, is_dwork);
 		wait_on_work(work);
 	} while (unlikely(ret < 0));
  
@@ -2853,7 +2870,7 @@
  */
 bool cancel_work_sync(struct work_struct *work)
 {
-	return __cancel_work_timer(work, NULL);
+	return __cancel_work_timer(work, false);
 }
 EXPORT_SYMBOL_GPL(cancel_work_sync);
  
@@ -2914,7 +2931,7 @@
  */
 bool cancel_delayed_work_sync(struct delayed_work *dwork)
 {
-	return __cancel_work_timer(&dwork->work, &dwork->timer);
+	return __cancel_work_timer(&dwork->work, true);
 }
 EXPORT_SYMBOL(cancel_delayed_work_sync);
...	...	@@ -1004,15 +1004,33 @@
1004	1004	complete(&cwq->wq->first_flusher->done);
1005	1005	}
1006	1006
1007		-/*
1008		- * Upon a successful return (>= 0), the caller "owns" WORK_STRUCT_PENDING bit,
1009		- * so this work can't be re-armed in any way.
	1007	+/**
	1008	+ * try_to_grab_pending - steal work item from worklist
	1009	+ * @work: work item to steal
	1010	+ * @is_dwork: @work is a delayed_work
	1011	+ *
	1012	+ * Try to grab PENDING bit of @work. This function can handle @work in any
	1013	+ * stable state - idle, on timer or on worklist. Return values are
	1014	+ *
	1015	+ * 1 if @work was pending and we successfully stole PENDING
	1016	+ * 0 if @work was idle and we claimed PENDING
	1017	+ * -EAGAIN if PENDING couldn't be grabbed at the moment, safe to busy-retry
	1018	+ *
	1019	+ * On >= 0 return, the caller owns @work's PENDING bit.
1010	1020	*/
1011		-static int try_to_grab_pending(struct work_struct *work)
	1021	+static int try_to_grab_pending(struct work_struct *work, bool is_dwork)
1012	1022	{
1013	1023	struct global_cwq *gcwq;
1014		- int ret = -1;
1015	1024
	1025	+ /* try to steal the timer if it exists */
	1026	+ if (is_dwork) {
	1027	+ struct delayed_work *dwork = to_delayed_work(work);
	1028	+
	1029	+ if (likely(del_timer(&dwork->timer)))
	1030	+ return 1;
	1031	+ }
	1032	+
	1033	+ /* try to claim PENDING the normal way */
1016	1034	if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work)))
1017	1035	return 0;
1018	1036
...	...	@@ -1022,7 +1040,7 @@
1022	1040	*/
1023	1041	gcwq = get_work_gcwq(work);
1024	1042	if (!gcwq)
1025		- return ret;
	1043	+ return -EAGAIN;
1026	1044
1027	1045	spin_lock_irq(&gcwq->lock);
1028	1046	if (!list_empty(&work->entry)) {
1029	1047
...	...	@@ -1038,12 +1056,14 @@
1038	1056	cwq_dec_nr_in_flight(get_work_cwq(work),
1039	1057	get_work_color(work),
1040	1058	*work_data_bits(work) & WORK_STRUCT_DELAYED);
1041		- ret = 1;
	1059	+
	1060	+ spin_unlock_irq(&gcwq->lock);
	1061	+ return 1;
1042	1062	}
1043	1063	}
1044	1064	spin_unlock_irq(&gcwq->lock);
1045	1065
1046		- return ret;
	1066	+ return -EAGAIN;
1047	1067	}
1048	1068
1049	1069	/**
1050	1070
...	...	@@ -2817,15 +2837,12 @@
2817	2837	}
2818	2838	EXPORT_SYMBOL_GPL(flush_work_sync);
2819	2839
2820		-static bool __cancel_work_timer(struct work_struct *work,
2821		- struct timer_list* timer)
	2840	+static bool __cancel_work_timer(struct work_struct *work, bool is_dwork)
2822	2841	{
2823	2842	int ret;
2824	2843
2825	2844	do {
2826		- ret = (timer && likely(del_timer(timer)));
2827		- if (!ret)
2828		- ret = try_to_grab_pending(work);
	2845	+ ret = try_to_grab_pending(work, is_dwork);
2829	2846	wait_on_work(work);
2830	2847	} while (unlikely(ret < 0));
2831	2848
...	...	@@ -2853,7 +2870,7 @@
2853	2870	*/
2854	2871	bool cancel_work_sync(struct work_struct *work)
2855	2872	{
2856		- return __cancel_work_timer(work, NULL);
	2873	+ return __cancel_work_timer(work, false);
2857	2874	}
2858	2875	EXPORT_SYMBOL_GPL(cancel_work_sync);
2859	2876
...	...	@@ -2914,7 +2931,7 @@
2914	2931	*/
2915	2932	bool cancel_delayed_work_sync(struct delayed_work *dwork)
2916	2933	{
2917		- return __cancel_work_timer(&dwork->work, &dwork->timer);
	2934	+ return __cancel_work_timer(&dwork->work, true);
2918	2935	}
2919	2936	EXPORT_SYMBOL(cancel_delayed_work_sync);
2920	2937