Message ID | 20250317235546.4546-5-dongli.zhang@oracle.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | vhost-scsi: log write descriptors for live migration (and three bugfix) | expand |
On Tue, Mar 18, 2025 at 7:51 AM Dongli Zhang <dongli.zhang@oracle.com> wrote: > > Currently, the only user of vhost_log_write() is vhost-net. The 'len' > argument prevents logging of pages that are not tainted by the RX path. > > Adjustments are needed since more drivers (i.e. vhost-scsi) begin using > vhost_log_write(). So far vhost-net RX path may only partially use pages > shared by the last vring descriptor. Unlike vhost-net, vhost-scsi always > logs all pages shared via vring descriptors. To accommodate this, a new > argument 'partial' is introduced. This argument works alongside 'len' to > indicate whether the driver should log all pages of a vring descriptor, or > only pages that are tainted by the driver. > > In addition, removes BUG(). > > Suggested-by: Joao Martins <joao.m.martins@oracle.com> > Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> > --- > drivers/vhost/net.c | 2 +- > drivers/vhost/vhost.c | 28 +++++++++++++++++----------- > drivers/vhost/vhost.h | 2 +- > 3 files changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index b9b9e9d40951..0e5d82bfde76 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -1219,7 +1219,7 @@ static void handle_rx(struct vhost_net *net) > if (nvq->done_idx > VHOST_NET_BATCH) > vhost_net_signal_used(nvq); > if (unlikely(vq_log)) > - vhost_log_write(vq, vq_log, log, vhost_len, > + vhost_log_write(vq, vq_log, log, vhost_len, true, > vq->iov, in); > total_len += vhost_len; > } while (likely(!vhost_exceeds_weight(vq, ++recv_pkts, total_len))); > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 9ac25d08f473..db3b30aba940 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2304,8 +2304,14 @@ static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len) > return 0; > } > > -int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, > - unsigned int log_num, u64 len, struct iovec *iov, int count) > +/* > + * 'len' is used only when 'partial' is true, to indicate whether the > + * entire length of each descriptor is logged. > + */ While at it, let's document all the parameters here. > +int vhost_log_write(struct vhost_virtqueue *vq, > + struct vhost_log *log, unsigned int log_num, > + u64 len, bool partial, > + struct iovec *iov, int count) > { > int i, r; > > @@ -2323,19 +2329,19 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, > } > > for (i = 0; i < log_num; ++i) { > - u64 l = min(log[i].len, len); > + u64 l = partial ? min(log[i].len, len) : log[i].len; > + > r = log_write(vq->log_base, log[i].addr, l); > if (r < 0) > return r; > - len -= l; > - if (!len) { > - if (vq->log_ctx) > - eventfd_signal(vq->log_ctx); > - return 0; > - } > + > + if (partial) > + len -= l; I wonder if it's simpler to just tweak the caller to call with the correct len (or probably U64_MAX) in this case? > } > - /* Length written exceeds what we have stored. This is a bug. */ > - BUG(); > + > + if (vq->log_ctx) > + eventfd_signal(vq->log_ctx); > + > return 0; > } > EXPORT_SYMBOL_GPL(vhost_log_write); > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index bb75a292d50c..5de5941988fe 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -224,7 +224,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *, struct vhost_virtqueue *); > bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *); > > int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, > - unsigned int log_num, u64 len, > + unsigned int log_num, u64 len, bool partial, > struct iovec *iov, int count); > int vq_meta_prefetch(struct vhost_virtqueue *vq); > > -- > 2.39.3 > Thanks
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index b9b9e9d40951..0e5d82bfde76 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -1219,7 +1219,7 @@ static void handle_rx(struct vhost_net *net) if (nvq->done_idx > VHOST_NET_BATCH) vhost_net_signal_used(nvq); if (unlikely(vq_log)) - vhost_log_write(vq, vq_log, log, vhost_len, + vhost_log_write(vq, vq_log, log, vhost_len, true, vq->iov, in); total_len += vhost_len; } while (likely(!vhost_exceeds_weight(vq, ++recv_pkts, total_len))); diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 9ac25d08f473..db3b30aba940 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2304,8 +2304,14 @@ static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len) return 0; } -int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, - unsigned int log_num, u64 len, struct iovec *iov, int count) +/* + * 'len' is used only when 'partial' is true, to indicate whether the + * entire length of each descriptor is logged. + */ +int vhost_log_write(struct vhost_virtqueue *vq, + struct vhost_log *log, unsigned int log_num, + u64 len, bool partial, + struct iovec *iov, int count) { int i, r; @@ -2323,19 +2329,19 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, } for (i = 0; i < log_num; ++i) { - u64 l = min(log[i].len, len); + u64 l = partial ? min(log[i].len, len) : log[i].len; + r = log_write(vq->log_base, log[i].addr, l); if (r < 0) return r; - len -= l; - if (!len) { - if (vq->log_ctx) - eventfd_signal(vq->log_ctx); - return 0; - } + + if (partial) + len -= l; } - /* Length written exceeds what we have stored. This is a bug. */ - BUG(); + + if (vq->log_ctx) + eventfd_signal(vq->log_ctx); + return 0; } EXPORT_SYMBOL_GPL(vhost_log_write); diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index bb75a292d50c..5de5941988fe 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -224,7 +224,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *, struct vhost_virtqueue *); bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *); int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, - unsigned int log_num, u64 len, + unsigned int log_num, u64 len, bool partial, struct iovec *iov, int count); int vq_meta_prefetch(struct vhost_virtqueue *vq);
Currently, the only user of vhost_log_write() is vhost-net. The 'len' argument prevents logging of pages that are not tainted by the RX path. Adjustments are needed since more drivers (i.e. vhost-scsi) begin using vhost_log_write(). So far vhost-net RX path may only partially use pages shared by the last vring descriptor. Unlike vhost-net, vhost-scsi always logs all pages shared via vring descriptors. To accommodate this, a new argument 'partial' is introduced. This argument works alongside 'len' to indicate whether the driver should log all pages of a vring descriptor, or only pages that are tainted by the driver. In addition, removes BUG(). Suggested-by: Joao Martins <joao.m.martins@oracle.com> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> --- drivers/vhost/net.c | 2 +- drivers/vhost/vhost.c | 28 +++++++++++++++++----------- drivers/vhost/vhost.h | 2 +- 3 files changed, 19 insertions(+), 13 deletions(-)