Re: [PATCH] nvme: fix return type of nvme_poll()

[Yongsoo Joo <ysjoo@kookmin.ac.kr>]

On Sat, Dec 07, 2024 at 08:13:12PM -0700, Keith Busch wrote:
> On Sat, Dec 07, 2024 at 04:55:46AM +0000, Yongsoo Joo wrote:
> > Change the type of the return variable 'found' from bool to int so
> > that any interested caller can see not only whether at least one
> > CQE was processed, but also the total number of CQEs processed.
> 
> It looks like all the interested callers only care if it's not 0, so I'm
> not sure this is a "fix" as the subject says.

I agree that all the "current" interested callers seem to expect 
boolean-like return values from nvme_poll(). However, the return 
type of both nvme_poll() and nvme_poll_cq() is int, and 
nvme_poll_cq() actually returns the number of processed CQEs.

The inconsistency between the two functions is, in my view, worth 
addressing to prevent potential confusion for potential callers who 
are interested the actual numbers. And this patch is 
backward-compatible with the current usage, so no changes are 
required for the existing callers.

I’d like to ask for your opinion on the followings:
- Do you think rephrasing the patch title to “nvme: change return 
  type of nvme_poll()” would be more appropriate?
- Do you believe it would make more sense to change the return 
  type of nvme_poll() from int to bool instead?
- Or, do you think this inconsistency is not a big issue, and the 
  current implementation is sufficient as it stands?

Thanks for your time and feedback. I look forward to your thoughts.


--