[GitHub] [incubator-brpc] wwbmmm commented on a diff in pull request #1751: butex_wake_all support nosched flag

[GitBox <gi...@apache.org>]

wwbmmm commented on code in PR #1751:
URL: https://github.com/apache/incubator-brpc/pull/1751#discussion_r857521566


##########
src/bthread/countdown_event.cpp:
##########
@@ -50,7 +50,7 @@ void CountdownEvent::signal(int sig) {
         return;
     }
     LOG_IF(ERROR, prev < sig) << "Counter is over decreased";
-    butex_wake_all(saved_butex);
+    butex_wake_all(saved_butex, true);

Review Comment:
   这里修改了框架的默认行为了，是不是不太合适
   可以给signal函数也加个nosched参数？



##########
src/bthread/butex.cpp:
##########
@@ -335,13 +347,18 @@ int butex_wake_all(void* arg) {
         g->ready_to_run_general(w->tid, true);
         ++nwakeup;
     }
-    if (saved_nwakeup != nwakeup) {
+    if (nosignal) {
+        tls_task_group_nosignal = g;
+        nwakeup = 0;
+    } else if (saved_nwakeup != nwakeup) {
         g->flush_nosignal_tasks_general();
     }
-    if (g == tls_task_group) {
-        TaskGroup::exchange(&g, next->tid);
-    } else {
-        g->ready_to_run_remote(next->tid);
+    if (!nosched) {
+        if (g == tls_task_group) {
+            TaskGroup::exchange(&g, next->tid);

Review Comment:
   这里是不是也要传nosignal



##########
src/bthread/butex.cpp:
##########
@@ -284,15 +286,19 @@ int butex_wake(void* arg, bool nosched) {
         if (!nosched) {
             TaskGroup::exchange(&g, bbw->tid);

Review Comment:
   exchange是不是也要传nosignal参数进去



##########
src/bthread/butex.cpp:
##########
@@ -319,11 +329,13 @@ int butex_wake_all(void* arg) {
         return nwakeup;
     }
     // We will exchange with first waiter in the end.
-    ButexBthreadWaiter* next = static_cast<ButexBthreadWaiter*>(
-        bthread_waiters.head()->value());
-    next->RemoveFromList();
-    unsleep_if_necessary(next, get_global_timer_thread());
-    ++nwakeup;
+    ButexBthreadWaiter* next =
+        static_cast<ButexBthreadWaiter*>(bthread_waiters.head()->value());
+    if (!nosched) {
+        next->RemoveFromList();
+        unsleep_if_necessary(next, get_global_timer_thread());
+        ++nwakeup;
+    }
     TaskGroup* g = get_task_group(next->control);

Review Comment:
   这里可能要先判断一下，如果nosignal且tls_task_group_nosignal不为空，则g = tls_task_group_nosignal



##########
src/bthread/butex.cpp:
##########
@@ -335,13 +347,18 @@ int butex_wake_all(void* arg) {
         g->ready_to_run_general(w->tid, true);
         ++nwakeup;
     }
-    if (saved_nwakeup != nwakeup) {
+    if (nosignal) {
+        tls_task_group_nosignal = g;
+        nwakeup = 0;
+    } else if (saved_nwakeup != nwakeup) {
         g->flush_nosignal_tasks_general();
     }
-    if (g == tls_task_group) {
-        TaskGroup::exchange(&g, next->tid);
-    } else {
-        g->ready_to_run_remote(next->tid);
+    if (!nosched) {
+        if (g == tls_task_group) {
+            TaskGroup::exchange(&g, next->tid);
+        } else {
+            g->ready_to_run_remote(next->tid);

Review Comment:
   同上



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org