xiaoxiang781216 commented on code in PR #20030:
URL: https://github.com/apache/nuttx/pull/20030#discussion_r3953468522


##########
drivers/serial/serial_io.c:
##########
@@ -57,24 +57,32 @@
 void uart_xmitchars(FAR uart_dev_t *dev)
 {
   uint16_t nbytes = 0;
+  sbuf_size_t head;
 
 #ifdef CONFIG_SMP
   irqstate_t flags = enter_critical_section();
 #endif
 
-  /* Send while we still have data in the TX buffer & room in the fifo */
+  /* Send while we still have data in the TX buffer & room in the fifo.
+   *
+   * uart_putxmitchar() advances xmit.head from thread context without
+   * holding the critical section, so on SMP it can move (and wrap) while

Review Comment:
   but the critical section is held at line 63



##########
sched/wqueue/kwork_cancel.c:
##########
@@ -84,6 +85,17 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, 
bool sync,
               work_timer_reset(wqueue);
             }
         }
+      else if (!sync)
+        {
+          /* The work is not queued: either it was never queued or a worker
+           * has already dequeued it and may be executing its callback right
+           * now.  Report -ENOENT so that callers (e.g. aio_cancel()) do not
+           * free resources that the callback is still using.  The sync
+           * variant waits for such a callback below and returns OK.
+           */
+
+          ret = -ENOENT;

Review Comment:
   need run the loop at line 102 to know the truth.



##########
libs/libc/aio/lio_listio.c:
##########
@@ -547,59 +580,61 @@ int lio_listio(int mode, FAR struct aiocb * const list[], 
int nent,
           status = OK;
           switch (aiocbp->aio_lio_opcode)
             {
-            case LIO_NOP:
-              {
-                /* Mark the do-nothing operation complete */
-
-                aiocbp->aio_result = OK;
-              }
-              break;
-
-            case LIO_READ:
-            case LIO_WRITE:
-              {
-                if (aiocbp->aio_lio_opcode == LIO_READ)
-                  {
-                    /* Submit the asynchronous read operation */
-
-                    status = aio_read(aiocbp);
-                  }
-                else
-                  {
-                    /* Submit the asynchronous write operation */
-
-                    status = aio_write(aiocbp);
-                  }
-
-                if (status < 0)
-                  {
-                    /* Failed to queue the I/O.  Set up the error return. */
-
-                    errcode = get_errno();
-                    ferr("ERROR: aio_read/write failed: %d\n", errcode);
-                    DEBUGASSERT(errcode > 0);
-                    aiocbp->aio_result = -errcode;
-                    ret = ERROR;
-                  }
-                else
-                  {
-                    /* Increment the count of successfully queue operations */
-
-                    nqueued++;
-                  }
-              }
-              break;
-
-            default:
-              {
-                /* Make the invalid operation complete with an error */
-
-                ferr("ERROR: Unrecognized opcode: %d\n",
-                     aiocbp->aio_lio_opcode);
-                aiocbp->aio_result = -EINVAL;
-                ret = ERROR;
-              }
-              break;
+              case LIO_NOP:

Review Comment:
   could you move format to new patch? since  it is hard to review the real 
change.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to