Thanks for the review, Viresh!
On Tue, Jun 9, 2026 at 3:35 AM Viresh Kumar <[email protected]> wrote:
> Maybe move the comment above the code ? Can be dropped too.
>
> Also, maybe there is a small race here, not sure. What if the other
> side (polls and) processes the message as soon as it is added to the
> queue with virtqueue_add_sgs() ? In that case virtio_i2c_msg_done()
> will call complete (which won't harm) and kref_put(). If this happens
> for the first req of the xfer, it may end up freeing the xfer while
> being used here ?
Good eye, thanks for the catch. Moved kref_get() to
before virtqueue_add_sgs()
> > -static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> > - struct virtio_i2c_req *reqs,
> > - struct i2c_msg *msgs, int num)
> > +static int virtio_i2c_complete_reqs(struct virtio_i2c_xfer *xfer)
>
> Maybe rename to complete_xfer now ?
Done
> > - if (!failed) {
> > - if
> > (wait_for_completion_interruptible(&req->completion))
> > - failed = true;
> > - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> > - failed = true;
> > - else
> > - j++;
> > + if (wait_for_completion_killable(&req->completion)) {
>
> Maybe do this in a separate patch ?
Good idea, reverted to wait_for_completion_interruptible()
> > - return j;
> > + return fail_index >= 0 ? fail_index : xfer->num; /* number of
> > successful transactions */
>
> If this comment is required, maybe add it above the line instead.
Done
