* [PATCH 0/3] vdpa_sim_blk: several fixes for the vDPA block simulator @ 2022-06-21 16:08 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization Cc: Jason Wang, Eugenio Pérez, Michael S. Tsirkin, linux-kernel, Stefano Garzarella The first two patches essentially limit the possibility of the guest doing a DoS to the host. The third makes the simulator more correct (following what we do in vdpa_sim_net) by calling vringh_complete_iotlb() in the error path as well. Stefano Garzarella (3): vdpa_sim_blk: use dev_dbg() to print errors vdpa_sim_blk: limit the number of request handled per batch vdpa_sim_blk: call vringh_complete_iotlb() also in the error path drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 49 +++++++++++++++++++--------- 1 file changed, 33 insertions(+), 16 deletions(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] vdpa_sim_blk: several fixes for the vDPA block simulator @ 2022-06-21 16:08 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization; +Cc: Eugenio Pérez, linux-kernel, Michael S. Tsirkin The first two patches essentially limit the possibility of the guest doing a DoS to the host. The third makes the simulator more correct (following what we do in vdpa_sim_net) by calling vringh_complete_iotlb() in the error path as well. Stefano Garzarella (3): vdpa_sim_blk: use dev_dbg() to print errors vdpa_sim_blk: limit the number of request handled per batch vdpa_sim_blk: call vringh_complete_iotlb() also in the error path drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 49 +++++++++++++++++++--------- 1 file changed, 33 insertions(+), 16 deletions(-) -- 2.36.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] vdpa_sim_blk: use dev_dbg() to print errors 2022-06-21 16:08 ` Stefano Garzarella @ 2022-06-21 16:08 ` Stefano Garzarella -1 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization Cc: Jason Wang, Eugenio Pérez, Michael S. Tsirkin, linux-kernel, Stefano Garzarella Use dev_dbg() instead of dev_err()/dev_warn() to avoid flooding the host with prints, when the guest driver is misbehaving. In this way, prints can be dynamically enabled when the vDPA block simulator is used to validate a driver. Suggested-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index 42d401d43911..a83a5c76f620 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -76,13 +76,13 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, return false; if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { - dev_err(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", + dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", vq->out_iov.used, vq->in_iov.used); return false; } if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { - dev_err(&vdpasim->vdpa.dev, "request in header too short\n"); + dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); return false; } @@ -96,7 +96,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, bytes = vringh_iov_pull_iotlb(&vq->vring, &vq->out_iov, &hdr, sizeof(hdr)); if (bytes != sizeof(hdr)) { - dev_err(&vdpasim->vdpa.dev, "request out header too short\n"); + dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); return false; } @@ -110,7 +110,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, switch (type) { case VIRTIO_BLK_T_IN: if (!vdpasim_blk_check_range(sector, to_push)) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "reading over the capacity - offset: 0x%llx len: 0x%zx\n", offset, to_push); status = VIRTIO_BLK_S_IOERR; @@ -121,7 +121,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim->buffer + offset, to_push); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_push_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", bytes, offset, to_push); status = VIRTIO_BLK_S_IOERR; @@ -133,7 +133,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, case VIRTIO_BLK_T_OUT: if (!vdpasim_blk_check_range(sector, to_pull)) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "writing over the capacity - offset: 0x%llx len: 0x%zx\n", offset, to_pull); status = VIRTIO_BLK_S_IOERR; @@ -144,7 +144,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim->buffer + offset, to_pull); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_pull_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", bytes, offset, to_pull); status = VIRTIO_BLK_S_IOERR; @@ -157,7 +157,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim_blk_id, VIRTIO_BLK_ID_BYTES); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_push_iotlb() error: %zd\n", bytes); status = VIRTIO_BLK_S_IOERR; break; @@ -167,8 +167,8 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, break; default: - dev_warn(&vdpasim->vdpa.dev, - "Unsupported request type %d\n", type); + dev_dbg(&vdpasim->vdpa.dev, + "Unsupported request type %d\n", type); status = VIRTIO_BLK_S_IOERR; break; } -- 2.36.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] vdpa_sim_blk: use dev_dbg() to print errors @ 2022-06-21 16:08 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization; +Cc: Eugenio Pérez, linux-kernel, Michael S. Tsirkin Use dev_dbg() instead of dev_err()/dev_warn() to avoid flooding the host with prints, when the guest driver is misbehaving. In this way, prints can be dynamically enabled when the vDPA block simulator is used to validate a driver. Suggested-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index 42d401d43911..a83a5c76f620 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -76,13 +76,13 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, return false; if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { - dev_err(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", + dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", vq->out_iov.used, vq->in_iov.used); return false; } if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { - dev_err(&vdpasim->vdpa.dev, "request in header too short\n"); + dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); return false; } @@ -96,7 +96,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, bytes = vringh_iov_pull_iotlb(&vq->vring, &vq->out_iov, &hdr, sizeof(hdr)); if (bytes != sizeof(hdr)) { - dev_err(&vdpasim->vdpa.dev, "request out header too short\n"); + dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); return false; } @@ -110,7 +110,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, switch (type) { case VIRTIO_BLK_T_IN: if (!vdpasim_blk_check_range(sector, to_push)) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "reading over the capacity - offset: 0x%llx len: 0x%zx\n", offset, to_push); status = VIRTIO_BLK_S_IOERR; @@ -121,7 +121,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim->buffer + offset, to_push); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_push_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", bytes, offset, to_push); status = VIRTIO_BLK_S_IOERR; @@ -133,7 +133,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, case VIRTIO_BLK_T_OUT: if (!vdpasim_blk_check_range(sector, to_pull)) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "writing over the capacity - offset: 0x%llx len: 0x%zx\n", offset, to_pull); status = VIRTIO_BLK_S_IOERR; @@ -144,7 +144,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim->buffer + offset, to_pull); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_pull_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", bytes, offset, to_pull); status = VIRTIO_BLK_S_IOERR; @@ -157,7 +157,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, vdpasim_blk_id, VIRTIO_BLK_ID_BYTES); if (bytes < 0) { - dev_err(&vdpasim->vdpa.dev, + dev_dbg(&vdpasim->vdpa.dev, "vringh_iov_push_iotlb() error: %zd\n", bytes); status = VIRTIO_BLK_S_IOERR; break; @@ -167,8 +167,8 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, break; default: - dev_warn(&vdpasim->vdpa.dev, - "Unsupported request type %d\n", type); + dev_dbg(&vdpasim->vdpa.dev, + "Unsupported request type %d\n", type); status = VIRTIO_BLK_S_IOERR; break; } -- 2.36.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] vdpa_sim_blk: use dev_dbg() to print errors 2022-06-21 16:08 ` Stefano Garzarella @ 2022-06-23 3:40 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:40 UTC (permalink / raw) To: Stefano Garzarella Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Use dev_dbg() instead of dev_err()/dev_warn() to avoid flooding the > host with prints, when the guest driver is misbehaving. > In this way, prints can be dynamically enabled when the vDPA block > simulator is used to validate a driver. > > Suggested-by: Jason Wang <jasowang@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index 42d401d43911..a83a5c76f620 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -76,13 +76,13 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > return false; > > if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { > - dev_err(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > + dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > vq->out_iov.used, vq->in_iov.used); > return false; > } > > if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { > - dev_err(&vdpasim->vdpa.dev, "request in header too short\n"); > + dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); > return false; > } > > @@ -96,7 +96,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > bytes = vringh_iov_pull_iotlb(&vq->vring, &vq->out_iov, &hdr, > sizeof(hdr)); > if (bytes != sizeof(hdr)) { > - dev_err(&vdpasim->vdpa.dev, "request out header too short\n"); > + dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); > return false; > } > > @@ -110,7 +110,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > switch (type) { > case VIRTIO_BLK_T_IN: > if (!vdpasim_blk_check_range(sector, to_push)) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "reading over the capacity - offset: 0x%llx len: 0x%zx\n", > offset, to_push); > status = VIRTIO_BLK_S_IOERR; > @@ -121,7 +121,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim->buffer + offset, > to_push); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_push_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", > bytes, offset, to_push); > status = VIRTIO_BLK_S_IOERR; > @@ -133,7 +133,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > > case VIRTIO_BLK_T_OUT: > if (!vdpasim_blk_check_range(sector, to_pull)) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "writing over the capacity - offset: 0x%llx len: 0x%zx\n", > offset, to_pull); > status = VIRTIO_BLK_S_IOERR; > @@ -144,7 +144,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim->buffer + offset, > to_pull); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_pull_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", > bytes, offset, to_pull); > status = VIRTIO_BLK_S_IOERR; > @@ -157,7 +157,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim_blk_id, > VIRTIO_BLK_ID_BYTES); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_push_iotlb() error: %zd\n", bytes); > status = VIRTIO_BLK_S_IOERR; > break; > @@ -167,8 +167,8 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > break; > > default: > - dev_warn(&vdpasim->vdpa.dev, > - "Unsupported request type %d\n", type); > + dev_dbg(&vdpasim->vdpa.dev, > + "Unsupported request type %d\n", type); > status = VIRTIO_BLK_S_IOERR; > break; > } > -- > 2.36.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] vdpa_sim_blk: use dev_dbg() to print errors @ 2022-06-23 3:40 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:40 UTC (permalink / raw) To: Stefano Garzarella Cc: virtualization, Eugenio Pérez, Michael S. Tsirkin, linux-kernel On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Use dev_dbg() instead of dev_err()/dev_warn() to avoid flooding the > host with prints, when the guest driver is misbehaving. > In this way, prints can be dynamically enabled when the vDPA block > simulator is used to validate a driver. > > Suggested-by: Jason Wang <jasowang@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index 42d401d43911..a83a5c76f620 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -76,13 +76,13 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > return false; > > if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { > - dev_err(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > + dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > vq->out_iov.used, vq->in_iov.used); > return false; > } > > if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { > - dev_err(&vdpasim->vdpa.dev, "request in header too short\n"); > + dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); > return false; > } > > @@ -96,7 +96,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > bytes = vringh_iov_pull_iotlb(&vq->vring, &vq->out_iov, &hdr, > sizeof(hdr)); > if (bytes != sizeof(hdr)) { > - dev_err(&vdpasim->vdpa.dev, "request out header too short\n"); > + dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); > return false; > } > > @@ -110,7 +110,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > switch (type) { > case VIRTIO_BLK_T_IN: > if (!vdpasim_blk_check_range(sector, to_push)) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "reading over the capacity - offset: 0x%llx len: 0x%zx\n", > offset, to_push); > status = VIRTIO_BLK_S_IOERR; > @@ -121,7 +121,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim->buffer + offset, > to_push); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_push_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", > bytes, offset, to_push); > status = VIRTIO_BLK_S_IOERR; > @@ -133,7 +133,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > > case VIRTIO_BLK_T_OUT: > if (!vdpasim_blk_check_range(sector, to_pull)) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "writing over the capacity - offset: 0x%llx len: 0x%zx\n", > offset, to_pull); > status = VIRTIO_BLK_S_IOERR; > @@ -144,7 +144,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim->buffer + offset, > to_pull); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_pull_iotlb() error: %zd offset: 0x%llx len: 0x%zx\n", > bytes, offset, to_pull); > status = VIRTIO_BLK_S_IOERR; > @@ -157,7 +157,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > vdpasim_blk_id, > VIRTIO_BLK_ID_BYTES); > if (bytes < 0) { > - dev_err(&vdpasim->vdpa.dev, > + dev_dbg(&vdpasim->vdpa.dev, > "vringh_iov_push_iotlb() error: %zd\n", bytes); > status = VIRTIO_BLK_S_IOERR; > break; > @@ -167,8 +167,8 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > break; > > default: > - dev_warn(&vdpasim->vdpa.dev, > - "Unsupported request type %d\n", type); > + dev_dbg(&vdpasim->vdpa.dev, > + "Unsupported request type %d\n", type); > status = VIRTIO_BLK_S_IOERR; > break; > } > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch 2022-06-21 16:08 ` Stefano Garzarella @ 2022-06-21 16:08 ` Stefano Garzarella -1 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization Cc: Jason Wang, Eugenio Pérez, Michael S. Tsirkin, linux-kernel, Stefano Garzarella Limit the number of requests (4 per queue as for vdpa_sim_net) handled in a batch to prevent the worker from using the CPU for too long. Suggested-by: Eugenio Pérez <eperezma@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index a83a5c76f620..ac86478845b6 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, static void vdpasim_blk_work(struct work_struct *work) { struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); + bool reschedule = false; int i; spin_lock(&vdpasim->lock); @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; + bool vq_work = true; + int reqs = 0; if (!vq->ready) continue; - while (vdpasim_blk_handle_req(vdpasim, vq)) { + while (vq_work) { + vq_work = vdpasim_blk_handle_req(vdpasim, vq); + /* Make sure used is visible before rasing the interrupt. */ smp_wmb(); @@ -218,10 +223,18 @@ static void vdpasim_blk_work(struct work_struct *work) if (vringh_need_notify_iotlb(&vq->vring) > 0) vringh_notify(&vq->vring); local_bh_enable(); + + if (++reqs > 4) { + vq_work = false; + reschedule = true; + } } } out: spin_unlock(&vdpasim->lock); + + if (reschedule) + schedule_work(&vdpasim->work); } static void vdpasim_blk_get_config(struct vdpasim *vdpasim, void *config) -- 2.36.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch @ 2022-06-21 16:08 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:08 UTC (permalink / raw) To: virtualization; +Cc: Eugenio Pérez, linux-kernel, Michael S. Tsirkin Limit the number of requests (4 per queue as for vdpa_sim_net) handled in a batch to prevent the worker from using the CPU for too long. Suggested-by: Eugenio Pérez <eperezma@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index a83a5c76f620..ac86478845b6 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, static void vdpasim_blk_work(struct work_struct *work) { struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); + bool reschedule = false; int i; spin_lock(&vdpasim->lock); @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; + bool vq_work = true; + int reqs = 0; if (!vq->ready) continue; - while (vdpasim_blk_handle_req(vdpasim, vq)) { + while (vq_work) { + vq_work = vdpasim_blk_handle_req(vdpasim, vq); + /* Make sure used is visible before rasing the interrupt. */ smp_wmb(); @@ -218,10 +223,18 @@ static void vdpasim_blk_work(struct work_struct *work) if (vringh_need_notify_iotlb(&vq->vring) > 0) vringh_notify(&vq->vring); local_bh_enable(); + + if (++reqs > 4) { + vq_work = false; + reschedule = true; + } } } out: spin_unlock(&vdpasim->lock); + + if (reschedule) + schedule_work(&vdpasim->work); } static void vdpasim_blk_get_config(struct vdpasim *vdpasim, void *config) -- 2.36.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch 2022-06-21 16:08 ` Stefano Garzarella @ 2022-06-23 3:50 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:50 UTC (permalink / raw) To: Stefano Garzarella Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Limit the number of requests (4 per queue as for vdpa_sim_net) handled > in a batch to prevent the worker from using the CPU for too long. > > Suggested-by: Eugenio Pérez <eperezma@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index a83a5c76f620..ac86478845b6 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > static void vdpasim_blk_work(struct work_struct *work) > { > struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > + bool reschedule = false; > int i; > > spin_lock(&vdpasim->lock); > @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > > for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > + bool vq_work = true; > + int reqs = 0; > > if (!vq->ready) > continue; > > - while (vdpasim_blk_handle_req(vdpasim, vq)) { > + while (vq_work) { > + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > + Is it better to check and exit the loop early here? Thanks > /* Make sure used is visible before rasing the interrupt. */ > smp_wmb(); > > @@ -218,10 +223,18 @@ static void vdpasim_blk_work(struct work_struct *work) > if (vringh_need_notify_iotlb(&vq->vring) > 0) > vringh_notify(&vq->vring); > local_bh_enable(); > + > + if (++reqs > 4) { > + vq_work = false; > + reschedule = true; > + } > } > } > out: > spin_unlock(&vdpasim->lock); > + > + if (reschedule) > + schedule_work(&vdpasim->work); > } > > static void vdpasim_blk_get_config(struct vdpasim *vdpasim, void *config) > -- > 2.36.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch @ 2022-06-23 3:50 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:50 UTC (permalink / raw) To: Stefano Garzarella Cc: virtualization, Eugenio Pérez, Michael S. Tsirkin, linux-kernel On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Limit the number of requests (4 per queue as for vdpa_sim_net) handled > in a batch to prevent the worker from using the CPU for too long. > > Suggested-by: Eugenio Pérez <eperezma@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index a83a5c76f620..ac86478845b6 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > static void vdpasim_blk_work(struct work_struct *work) > { > struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > + bool reschedule = false; > int i; > > spin_lock(&vdpasim->lock); > @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > > for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > + bool vq_work = true; > + int reqs = 0; > > if (!vq->ready) > continue; > > - while (vdpasim_blk_handle_req(vdpasim, vq)) { > + while (vq_work) { > + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > + Is it better to check and exit the loop early here? Thanks > /* Make sure used is visible before rasing the interrupt. */ > smp_wmb(); > > @@ -218,10 +223,18 @@ static void vdpasim_blk_work(struct work_struct *work) > if (vringh_need_notify_iotlb(&vq->vring) > 0) > vringh_notify(&vq->vring); > local_bh_enable(); > + > + if (++reqs > 4) { > + vq_work = false; > + reschedule = true; > + } > } > } > out: > spin_unlock(&vdpasim->lock); > + > + if (reschedule) > + schedule_work(&vdpasim->work); > } > > static void vdpasim_blk_get_config(struct vdpasim *vdpasim, void *config) > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch 2022-06-23 3:50 ` Jason Wang @ 2022-06-23 8:58 ` Stefano Garzarella -1 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-23 8:58 UTC (permalink / raw) To: Jason Wang Cc: virtualization, Eugenio Pérez, Michael S. Tsirkin, linux-kernel On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: >> >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled >> in a batch to prevent the worker from using the CPU for too long. >> >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> index a83a5c76f620..ac86478845b6 100644 >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, >> static void vdpasim_blk_work(struct work_struct *work) >> { >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); >> + bool reschedule = false; >> int i; >> >> spin_lock(&vdpasim->lock); >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) >> >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; >> + bool vq_work = true; >> + int reqs = 0; >> >> if (!vq->ready) >> continue; >> >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { >> + while (vq_work) { >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); >> + > >Is it better to check and exit the loop early here? Maybe, but I'm not sure. In vdpa_sim_net we call vringh_complete_iotlb() and send notification also in the error path, so I thought was better to send notification also when vdpasim_blk_handle_req() return false, since we will update the used.idx. However, I don't think it's a common path, so if you think it's better to exit the loop early, I can do it. Thanks, Stefano ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch @ 2022-06-23 8:58 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-23 8:58 UTC (permalink / raw) To: Jason Wang Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: >> >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled >> in a batch to prevent the worker from using the CPU for too long. >> >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> index a83a5c76f620..ac86478845b6 100644 >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, >> static void vdpasim_blk_work(struct work_struct *work) >> { >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); >> + bool reschedule = false; >> int i; >> >> spin_lock(&vdpasim->lock); >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) >> >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; >> + bool vq_work = true; >> + int reqs = 0; >> >> if (!vq->ready) >> continue; >> >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { >> + while (vq_work) { >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); >> + > >Is it better to check and exit the loop early here? Maybe, but I'm not sure. In vdpa_sim_net we call vringh_complete_iotlb() and send notification also in the error path, so I thought was better to send notification also when vdpasim_blk_handle_req() return false, since we will update the used.idx. However, I don't think it's a common path, so if you think it's better to exit the loop early, I can do it. Thanks, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch 2022-06-23 8:58 ` Stefano Garzarella @ 2022-06-28 4:01 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-28 4:01 UTC (permalink / raw) To: Stefano Garzarella Cc: virtualization, Eugenio Pérez, Michael S. Tsirkin, linux-kernel On Thu, Jun 23, 2022 at 4:58 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: > >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > >> > >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled > >> in a batch to prevent the worker from using the CPU for too long. > >> > >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >> --- > >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > >> 1 file changed, 14 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> index a83a5c76f620..ac86478845b6 100644 > >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > >> static void vdpasim_blk_work(struct work_struct *work) > >> { > >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > >> + bool reschedule = false; > >> int i; > >> > >> spin_lock(&vdpasim->lock); > >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > >> > >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > >> + bool vq_work = true; > >> + int reqs = 0; > >> > >> if (!vq->ready) > >> continue; > >> > >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { > >> + while (vq_work) { > >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > >> + > > > >Is it better to check and exit the loop early here? > > Maybe, but I'm not sure. > > In vdpa_sim_net we call vringh_complete_iotlb() and send notification > also in the error path, Looks not? read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl, sizeof(ctrl)); if (read != sizeof(ctrl)) break; We break the loop. Thanks > so I thought was better to send notification > also when vdpasim_blk_handle_req() return false, since we will update > the used.idx. > > However, I don't think it's a common path, so if you think it's better > to exit the loop early, I can do it. > > Thanks, > Stefano > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch @ 2022-06-28 4:01 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-28 4:01 UTC (permalink / raw) To: Stefano Garzarella Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Thu, Jun 23, 2022 at 4:58 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: > >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > >> > >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled > >> in a batch to prevent the worker from using the CPU for too long. > >> > >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >> --- > >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > >> 1 file changed, 14 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> index a83a5c76f620..ac86478845b6 100644 > >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > >> static void vdpasim_blk_work(struct work_struct *work) > >> { > >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > >> + bool reschedule = false; > >> int i; > >> > >> spin_lock(&vdpasim->lock); > >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > >> > >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > >> + bool vq_work = true; > >> + int reqs = 0; > >> > >> if (!vq->ready) > >> continue; > >> > >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { > >> + while (vq_work) { > >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > >> + > > > >Is it better to check and exit the loop early here? > > Maybe, but I'm not sure. > > In vdpa_sim_net we call vringh_complete_iotlb() and send notification > also in the error path, Looks not? read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl, sizeof(ctrl)); if (read != sizeof(ctrl)) break; We break the loop. Thanks > so I thought was better to send notification > also when vdpasim_blk_handle_req() return false, since we will update > the used.idx. > > However, I don't think it's a common path, so if you think it's better > to exit the loop early, I can do it. > > Thanks, > Stefano > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch 2022-06-28 4:01 ` Jason Wang @ 2022-06-28 7:55 ` Stefano Garzarella -1 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-28 7:55 UTC (permalink / raw) To: Jason Wang Cc: virtualization, Eugenio Pérez, Michael S. Tsirkin, linux-kernel On Tue, Jun 28, 2022 at 6:01 AM Jason Wang <jasowang@redhat.com> wrote: > > On Thu, Jun 23, 2022 at 4:58 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: > > >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > >> > > >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled > > >> in a batch to prevent the worker from using the CPU for too long. > > >> > > >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> > > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > >> --- > > >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > > >> 1 file changed, 14 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> index a83a5c76f620..ac86478845b6 100644 > > >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > > >> static void vdpasim_blk_work(struct work_struct *work) > > >> { > > >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > > >> + bool reschedule = false; > > >> int i; > > >> > > >> spin_lock(&vdpasim->lock); > > >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > > >> > > >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > > >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > > >> + bool vq_work = true; > > >> + int reqs = 0; > > >> > > >> if (!vq->ready) > > >> continue; > > >> > > >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { > > >> + while (vq_work) { > > >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > > >> + > > > > > >Is it better to check and exit the loop early here? > > > > Maybe, but I'm not sure. > > > > In vdpa_sim_net we call vringh_complete_iotlb() and send notification > > also in the error path, > > Looks not? > > read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl, > sizeof(ctrl)); > if (read != sizeof(ctrl)) > break; > > We break the loop. I was looking at vdpasim_net_work(), but I was confused since it handles 2 queues. I'll break the loop as it was before. Thanks, Stefano ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch @ 2022-06-28 7:55 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-28 7:55 UTC (permalink / raw) To: Jason Wang Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Tue, Jun 28, 2022 at 6:01 AM Jason Wang <jasowang@redhat.com> wrote: > > On Thu, Jun 23, 2022 at 4:58 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > On Thu, Jun 23, 2022 at 11:50:22AM +0800, Jason Wang wrote: > > >On Wed, Jun 22, 2022 at 12:09 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > >> > > >> Limit the number of requests (4 per queue as for vdpa_sim_net) handled > > >> in a batch to prevent the worker from using the CPU for too long. > > >> > > >> Suggested-by: Eugenio Pérez <eperezma@redhat.com> > > >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > >> --- > > >> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 15 ++++++++++++++- > > >> 1 file changed, 14 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> index a83a5c76f620..ac86478845b6 100644 > > >> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > > >> @@ -197,6 +197,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > > >> static void vdpasim_blk_work(struct work_struct *work) > > >> { > > >> struct vdpasim *vdpasim = container_of(work, struct vdpasim, work); > > >> + bool reschedule = false; > > >> int i; > > >> > > >> spin_lock(&vdpasim->lock); > > >> @@ -206,11 +207,15 @@ static void vdpasim_blk_work(struct work_struct *work) > > >> > > >> for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) { > > >> struct vdpasim_virtqueue *vq = &vdpasim->vqs[i]; > > >> + bool vq_work = true; > > >> + int reqs = 0; > > >> > > >> if (!vq->ready) > > >> continue; > > >> > > >> - while (vdpasim_blk_handle_req(vdpasim, vq)) { > > >> + while (vq_work) { > > >> + vq_work = vdpasim_blk_handle_req(vdpasim, vq); > > >> + > > > > > >Is it better to check and exit the loop early here? > > > > Maybe, but I'm not sure. > > > > In vdpa_sim_net we call vringh_complete_iotlb() and send notification > > also in the error path, > > Looks not? > > read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->in_iov, &ctrl, > sizeof(ctrl)); > if (read != sizeof(ctrl)) > break; > > We break the loop. I was looking at vdpasim_net_work(), but I was confused since it handles 2 queues. I'll break the loop as it was before. Thanks, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path 2022-06-21 16:08 ` Stefano Garzarella @ 2022-06-21 16:12 ` Stefano Garzarella -1 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:12 UTC (permalink / raw) To: virtualization Cc: Michael S. Tsirkin, linux-kernel, Jason Wang, Eugenio Pérez, Stefano Garzarella Call vringh_complete_iotlb() even when we encounter a serious error that prevents us from writing the status in the "in" header (e.g. the header length is incorrect, etc.). The guest is misbehaving, so maybe the ring is in a bad state, but let's avoid making things worse. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index ac86478845b6..de9cd9843143 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, { size_t pushed = 0, to_pull, to_push; struct virtio_blk_outhdr hdr; + bool handled = false; ssize_t bytes; loff_t offset; u64 sector; @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", vq->out_iov.used, vq->in_iov.used); - return false; + goto err; } if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); - return false; + goto err; } /* The last byte is the status and we checked if the last iov has @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, sizeof(hdr)); if (bytes != sizeof(hdr)) { dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); - return false; + goto err; } to_pull -= bytes; @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, /* Last byte is the status */ bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1); if (bytes != 1) - return false; + goto err; pushed += bytes; /* Make sure data is wrote before advancing index */ smp_wmb(); + handled = true; + +err: vringh_complete_iotlb(&vq->vring, vq->head, pushed); - return true; + return handled; } static void vdpasim_blk_work(struct work_struct *work) -- 2.36.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path @ 2022-06-21 16:12 ` Stefano Garzarella 0 siblings, 0 replies; 20+ messages in thread From: Stefano Garzarella @ 2022-06-21 16:12 UTC (permalink / raw) To: virtualization; +Cc: Eugenio Pérez, linux-kernel, Michael S. Tsirkin Call vringh_complete_iotlb() even when we encounter a serious error that prevents us from writing the status in the "in" header (e.g. the header length is incorrect, etc.). The guest is misbehaving, so maybe the ring is in a bad state, but let's avoid making things worse. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c index ac86478845b6..de9cd9843143 100644 --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, { size_t pushed = 0, to_pull, to_push; struct virtio_blk_outhdr hdr; + bool handled = false; ssize_t bytes; loff_t offset; u64 sector; @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", vq->out_iov.used, vq->in_iov.used); - return false; + goto err; } if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); - return false; + goto err; } /* The last byte is the status and we checked if the last iov has @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, sizeof(hdr)); if (bytes != sizeof(hdr)) { dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); - return false; + goto err; } to_pull -= bytes; @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, /* Last byte is the status */ bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1); if (bytes != 1) - return false; + goto err; pushed += bytes; /* Make sure data is wrote before advancing index */ smp_wmb(); + handled = true; + +err: vringh_complete_iotlb(&vq->vring, vq->head, pushed); - return true; + return handled; } static void vdpasim_blk_work(struct work_struct *work) -- 2.36.1 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path 2022-06-21 16:12 ` Stefano Garzarella @ 2022-06-23 3:52 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:52 UTC (permalink / raw) To: Stefano Garzarella Cc: Eugenio Pérez, Michael S. Tsirkin, linux-kernel, virtualization On Wed, Jun 22, 2022 at 12:13 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Call vringh_complete_iotlb() even when we encounter a serious error > that prevents us from writing the status in the "in" header > (e.g. the header length is incorrect, etc.). > > The guest is misbehaving, so maybe the ring is in a bad state, but > let's avoid making things worse. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index ac86478845b6..de9cd9843143 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > { > size_t pushed = 0, to_pull, to_push; > struct virtio_blk_outhdr hdr; > + bool handled = false; > ssize_t bytes; > loff_t offset; > u64 sector; > @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { > dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > vq->out_iov.used, vq->in_iov.used); > - return false; > + goto err; > } > > if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { > dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); > - return false; > + goto err; > } > > /* The last byte is the status and we checked if the last iov has > @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > sizeof(hdr)); > if (bytes != sizeof(hdr)) { > dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); > - return false; > + goto err; > } > > to_pull -= bytes; > @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > /* Last byte is the status */ > bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1); > if (bytes != 1) > - return false; > + goto err; > > pushed += bytes; > > /* Make sure data is wrote before advancing index */ > smp_wmb(); > > + handled = true; > + > +err: > vringh_complete_iotlb(&vq->vring, vq->head, pushed); > > - return true; > + return handled; > } > > static void vdpasim_blk_work(struct work_struct *work) > -- > 2.36.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path @ 2022-06-23 3:52 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2022-06-23 3:52 UTC (permalink / raw) To: Stefano Garzarella Cc: virtualization, Michael S. Tsirkin, linux-kernel, Eugenio Pérez On Wed, Jun 22, 2022 at 12:13 AM Stefano Garzarella <sgarzare@redhat.com> wrote: > > Call vringh_complete_iotlb() even when we encounter a serious error > that prevents us from writing the status in the "in" header > (e.g. the header length is incorrect, etc.). > > The guest is misbehaving, so maybe the ring is in a bad state, but > let's avoid making things worse. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index ac86478845b6..de9cd9843143 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > { > size_t pushed = 0, to_pull, to_push; > struct virtio_blk_outhdr hdr; > + bool handled = false; > ssize_t bytes; > loff_t offset; > u64 sector; > @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > if (vq->out_iov.used < 1 || vq->in_iov.used < 1) { > dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n", > vq->out_iov.used, vq->in_iov.used); > - return false; > + goto err; > } > > if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) { > dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n"); > - return false; > + goto err; > } > > /* The last byte is the status and we checked if the last iov has > @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > sizeof(hdr)); > if (bytes != sizeof(hdr)) { > dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n"); > - return false; > + goto err; > } > > to_pull -= bytes; > @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim, > /* Last byte is the status */ > bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1); > if (bytes != 1) > - return false; > + goto err; > > pushed += bytes; > > /* Make sure data is wrote before advancing index */ > smp_wmb(); > > + handled = true; > + > +err: > vringh_complete_iotlb(&vq->vring, vq->head, pushed); > > - return true; > + return handled; > } > > static void vdpasim_blk_work(struct work_struct *work) > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-06-28 7:55 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-21 16:08 [PATCH 0/3] vdpa_sim_blk: several fixes for the vDPA block simulator Stefano Garzarella 2022-06-21 16:08 ` Stefano Garzarella 2022-06-21 16:08 ` [PATCH 1/3] vdpa_sim_blk: use dev_dbg() to print errors Stefano Garzarella 2022-06-21 16:08 ` Stefano Garzarella 2022-06-23 3:40 ` Jason Wang 2022-06-23 3:40 ` Jason Wang 2022-06-21 16:08 ` [PATCH 2/3] vdpa_sim_blk: limit the number of request handled per batch Stefano Garzarella 2022-06-21 16:08 ` Stefano Garzarella 2022-06-23 3:50 ` Jason Wang 2022-06-23 3:50 ` Jason Wang 2022-06-23 8:58 ` Stefano Garzarella 2022-06-23 8:58 ` Stefano Garzarella 2022-06-28 4:01 ` Jason Wang 2022-06-28 4:01 ` Jason Wang 2022-06-28 7:55 ` Stefano Garzarella 2022-06-28 7:55 ` Stefano Garzarella 2022-06-21 16:12 ` [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path Stefano Garzarella 2022-06-21 16:12 ` Stefano Garzarella 2022-06-23 3:52 ` Jason Wang 2022-06-23 3:52 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.