linux-kernel - [PATCH v3] usb: gadget: uvc: clean up comments and styling in video

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230602220455.313801-1-arakesh@google.com>
Date:   Fri,  2 Jun 2023 15:04:55 -0700
From:   Avichal Rakesh <arakesh@...gle.com>
To:     dan.scally@...asonboard.com, laurent.pinchart@...asonboard.com
Cc:     thinh.nguyen@...opsys.com, arakesh@...gle.com, etalvala@...gle.com,
        gregkh@...uxfoundation.org, jchowdhary@...gle.com,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: [PATCH v3] usb: gadget: uvc: clean up comments and styling in video_pump

This patch elaborates on some of the edge cases handled by
video_pump around setting no_interrupt flag, and brings the
code style in line with rest of the file.

Link: https://lore.kernel.org/20230602151916.GH26944@pendragon.ideasonboard.com/
Signed-off-by: Avichal Rakesh <arakesh@...gle.com>
---
Changelog:
v2:
  - Updated commit message to make it clear that userspace application is not
    required to match the ISOC rate.
  - Styling and comment revision based on review
v3:
  - Rebased on to Greg's usb-next where v1 had already merged
  - Updated commit message to match the actual changes after rebase.


 drivers/usb/gadget/function/uvc_video.c | 38 ++++++++++++++++---------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index e81865978299..91af3b1ef0d4 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -382,13 +382,13 @@ static void uvcg_video_pump(struct work_struct *work)
 {
 	struct uvc_video *video = container_of(work, struct uvc_video, pump);
 	struct uvc_video_queue *queue = &video->queue;
+	/* video->max_payload_size is only set when using bulk transfer */
+	bool is_bulk = video->max_payload_size;
 	struct usb_request *req = NULL;
 	struct uvc_buffer *buf;
 	unsigned long flags;
+	bool buf_done;
 	int ret;
-	bool buf_int;
-	/* video->max_payload_size is only set when using bulk transfer */
-	bool is_bulk = video->max_payload_size;

 	while (video->ep->enabled) {
 		/*
@@ -414,20 +414,19 @@ static void uvcg_video_pump(struct work_struct *work)

 		if (buf != NULL) {
 			video->encode(req, video, buf);
-			/* Always interrupt for the last request of a video buffer */
-			buf_int = buf->state == UVC_BUF_STATE_DONE;
+			buf_done = buf->state == UVC_BUF_STATE_DONE;
 		} else if (!(queue->flags & UVC_QUEUE_DISCONNECTED) && !is_bulk) {
 			/*
 			 * No video buffer available; the queue is still connected and
-			 * we're traferring over ISOC. Queue a 0 length request to
+			 * we're transferring over ISOC. Queue a 0 length request to
 			 * prevent missed ISOC transfers.
 			 */
 			req->length = 0;
-			buf_int = false;
+			buf_done = false;
 		} else {
 			/*
-			 * Either queue has been disconnected or no video buffer
-			 * available to bulk transfer. Either way, stop processing
+			 * Either the queue has been disconnected or no video buffer
+			 * available for bulk transfer. Either way, stop processing
 			 * further.
 			 */
 			spin_unlock_irqrestore(&queue->irqlock, flags);
@@ -435,11 +434,24 @@ static void uvcg_video_pump(struct work_struct *work)
 		}

 		/*
-		 * With usb3 we have more requests. This will decrease the
-		 * interrupt load to a quarter but also catches the corner
-		 * cases, which needs to be handled.
+		 * With USB3 handling more requests at a higher speed, we can't
+		 * afford to generate an interrupt for every request. Decide to
+		 * interrupt:
+		 *
+		 * - When no more requests are available in the free queue, as
+		 *   this may be our last chance to refill the endpoint's
+		 *   request queue.
+		 *
+		 * - When this is request is the last request for the video
+		 *   buffer, as we want to start sending the next video buffer
+		 *   ASAP in case it doesn't get started already in the next
+		 *   iteration of this loop.
+		 *
+		 * - Four times over the length of the requests queue (as
+		 *   indicated by video->uvc_num_requests), as a trade-off
+		 *   between latency and interrupt load.
 		 */
-		if (list_empty(&video->req_free) || buf_int ||
+		if (list_empty(&video->req_free) || buf_done ||
 		    !(video->req_int_count %
 		       DIV_ROUND_UP(video->uvc_num_requests, 4))) {
 			video->req_int_count = 0;
--
2.41.0.rc0.172.g3f132b7071-goog