[PATCH v2] nvme: change the return type of nvme_poll()

[Keith Busch]

On Mon, Dec 09, 2024 at 09:39:42PM +0000, Yongsoo Joo wrote:
> On Mon, Dec 09, 2024 at 08:44:28AM -0700, Keith Busch wrote:
> > On Mon, Dec 09, 2024 at 01:33:44PM +0000, Yongsoo Joo wrote:
> > > -static int nvme_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
> > > +static bool nvme_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
> > >  {
> > >  	struct nvme_queue *nvmeq = hctx->driver_data;
> > >  	bool found;
> > > -- 
> > 
> > This function is registered with blk_mq_ops, so you would have to change
> > the prototype there too, and every other driver that also registers a
> > poll op. Honestly, it may not be worth the effort.
> Oh, I overlooked the issue you pointed out, which brings me back to my initial 
> suggestion. The function nvme_poll_cq() returns the number of found CQEs, but 
> nvme_poll() seems to unnecessarily reduce this information to a boolean.

Can we just change nvme_poll_cq() instead to return a bool instead?

---
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 4c644bb7f0692..54910f8eadff8 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1148,13 +1148,13 @@ static inline void nvme_update_cq_head(struct nvme_queue *nvmeq)
 	}
 }
 
-static inline int nvme_poll_cq(struct nvme_queue *nvmeq,
-			       struct io_comp_batch *iob)
+static inline bool nvme_poll_cq(struct nvme_queue *nvmeq,
+				struct io_comp_batch *iob)
 {
-	int found = 0;
+	bool found = false;
 
 	while (nvme_cqe_pending(nvmeq)) {
-		found++;
+		found = true
 		/*
 		 * load-load control dependency between phase and the rest of
 		 * the cqe requires a full read memory barrier
--

> A minimal change from "bool found" to "int found" in nvme_poll() would 
> preserve this information, allowing any interested researcher to make use of 
> it. Also, the description of poll in blk_mq_ops can be expanded as follows:
>        /**
>         * @poll: Called to poll for completion of a specific tag.
>         * Returns the number of CQEs found.
>         */

Since you pointed this comment out, I see it is from an earlier versions
of this interface and outdated. The current behavior polls for any
completion rather than a specific tag.