| Message ID | 20191003201858.11666-8-dave@stgolabs.net (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | lib/interval-tree: move to half closed intervals | expand |
On Thu, Oct 03, 2019 at 01:18:54PM -0700, Davidlohr Bueso wrote: > @@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq, > { > const struct vhost_umem_node *node; > struct vhost_umem *umem = vq->iotlb; > - u64 s = 0, size, orig_addr = addr, last = addr + len - 1; > + u64 s = 0, size, orig_addr = addr, last = addr + len; maybe "end" or "end_addr" instead of "last". > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index e9ed2722b633..bb36cb9ed5ec 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -53,13 +53,13 @@ struct vhost_log { > }; > > #define START(node) ((node)->start) > -#define LAST(node) ((node)->last) > +#define END(node) ((node)->end) > > struct vhost_umem_node { > struct rb_node rb; > struct list_head link; > __u64 start; > - __u64 last; > + __u64 end; > __u64 size; > __u64 userspace_addr; > __u32 perm; Preferably also rename __subtree_last to __subtree_end Looks good to me otherwise.
On Fri, 04 Oct 2019, Michel Lespinasse wrote: >On Thu, Oct 03, 2019 at 01:18:54PM -0700, Davidlohr Bueso wrote: >> @@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq, >> { >> const struct vhost_umem_node *node; >> struct vhost_umem *umem = vq->iotlb; >> - u64 s = 0, size, orig_addr = addr, last = addr + len - 1; >> + u64 s = 0, size, orig_addr = addr, last = addr + len; > >maybe "end" or "end_addr" instead of "last". > >> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h >> index e9ed2722b633..bb36cb9ed5ec 100644 >> --- a/drivers/vhost/vhost.h >> +++ b/drivers/vhost/vhost.h >> @@ -53,13 +53,13 @@ struct vhost_log { >> }; >> >> #define START(node) ((node)->start) >> -#define LAST(node) ((node)->last) >> +#define END(node) ((node)->end) >> >> struct vhost_umem_node { >> struct rb_node rb; >> struct list_head link; >> __u64 start; >> - __u64 last; >> + __u64 end; >> __u64 size; >> __u64 userspace_addr; >> __u32 perm; > >Preferably also rename __subtree_last to __subtree_end Yes, this was was another one that I had in mind renaming, but didn't want to grow the series -- all custom interval trees name _last for the subtree iirc. Like my previous reply, I'd rather leave this stuff for a followup series. Thanks, Davidlohr
On 2019/10/4 上午4:18, Davidlohr Bueso wrote: > The vhost_umem interval tree really wants [a, b) intervals, > not fully closed as currently. As such convert it to use the > new interval_tree_gen.h, and also rename the 'last' endpoint > in the node to 'end', which both a more suitable name for > the half closed interval and also reduces the chances of some > caller being missed. > > Cc: Michael S. Tsirkin" <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: virtualization@lists.linux-foundation.org > Signed-off-by: Davidlohr Bueso <dbueso@suse.de> > --- > drivers/vhost/vhost.c | 19 +++++++++---------- > drivers/vhost/vhost.h | 4 ++-- > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 36ca2cf419bf..80c3cca24dc7 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -28,7 +28,7 @@ > #include <linux/sort.h> > #include <linux/sched/mm.h> > #include <linux/sched/signal.h> > -#include <linux/interval_tree_generic.h> > +#include <linux/interval_tree_gen.h> > #include <linux/nospec.h> > > #include "vhost.h" > @@ -51,7 +51,7 @@ enum { > > INTERVAL_TREE_DEFINE(struct vhost_umem_node, > rb, __u64, __subtree_last, > - START, LAST, static inline, vhost_umem_interval_tree); > + START, END, static inline, vhost_umem_interval_tree); > > #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY > static void vhost_disable_cross_endian(struct vhost_virtqueue *vq) > @@ -1034,7 +1034,7 @@ static int vhost_new_umem_range(struct vhost_umem *umem, > > node->start = start; > node->size = size; > - node->last = end; > + node->end = end; > node->userspace_addr = userspace_addr; > node->perm = perm; > INIT_LIST_HEAD(&node->link); > @@ -1112,7 +1112,7 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, > } > vhost_vq_meta_reset(dev); > if (vhost_new_umem_range(dev->iotlb, msg->iova, msg->size, > - msg->iova + msg->size - 1, > + msg->iova + msg->size, > msg->uaddr, msg->perm)) { > ret = -ENOMEM; > break; > @@ -1126,7 +1126,7 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, > } > vhost_vq_meta_reset(dev); > vhost_del_umem_range(dev->iotlb, msg->iova, > - msg->iova + msg->size - 1); > + msg->iova + msg->size); > break; > default: > ret = -EINVAL; > @@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq, > { > const struct vhost_umem_node *node; > struct vhost_umem *umem = vq->iotlb; > - u64 s = 0, size, orig_addr = addr, last = addr + len - 1; > + u64 s = 0, size, orig_addr = addr, last = addr + len; > > if (vhost_vq_meta_fetch(vq, addr, len, type)) > return true; > > while (len > s) { > node = vhost_umem_interval_tree_iter_first(&umem->umem_tree, > - addr, > - last); > + addr, last); > if (node == NULL || node->start > addr) { > vhost_iotlb_miss(vq, addr, access); > return false; > @@ -1455,7 +1454,7 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) > region->guest_phys_addr, > region->memory_size, > region->guest_phys_addr + > - region->memory_size - 1, > + region->memory_size, > region->userspace_addr, > VHOST_ACCESS_RW)) > goto err; > @@ -2055,7 +2054,7 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, > } > > node = vhost_umem_interval_tree_iter_first(&umem->umem_tree, > - addr, addr + len - 1); > + addr, addr + len); > if (node == NULL || node->start > addr) { > if (umem != dev->iotlb) { > ret = -EFAULT; > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > index e9ed2722b633..bb36cb9ed5ec 100644 > --- a/drivers/vhost/vhost.h > +++ b/drivers/vhost/vhost.h > @@ -53,13 +53,13 @@ struct vhost_log { > }; > > #define START(node) ((node)->start) > -#define LAST(node) ((node)->last) > +#define END(node) ((node)->end) > > struct vhost_umem_node { > struct rb_node rb; > struct list_head link; > __u64 start; > - __u64 last; > + __u64 end; > __u64 size; > __u64 userspace_addr; > __u32 perm; Reviewed-by: Jason Wang <jasowang@redhat.com>
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 36ca2cf419bf..80c3cca24dc7 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -28,7 +28,7 @@ #include <linux/sort.h> #include <linux/sched/mm.h> #include <linux/sched/signal.h> -#include <linux/interval_tree_generic.h> +#include <linux/interval_tree_gen.h> #include <linux/nospec.h> #include "vhost.h" @@ -51,7 +51,7 @@ enum { INTERVAL_TREE_DEFINE(struct vhost_umem_node, rb, __u64, __subtree_last, - START, LAST, static inline, vhost_umem_interval_tree); + START, END, static inline, vhost_umem_interval_tree); #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY static void vhost_disable_cross_endian(struct vhost_virtqueue *vq) @@ -1034,7 +1034,7 @@ static int vhost_new_umem_range(struct vhost_umem *umem, node->start = start; node->size = size; - node->last = end; + node->end = end; node->userspace_addr = userspace_addr; node->perm = perm; INIT_LIST_HEAD(&node->link); @@ -1112,7 +1112,7 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, } vhost_vq_meta_reset(dev); if (vhost_new_umem_range(dev->iotlb, msg->iova, msg->size, - msg->iova + msg->size - 1, + msg->iova + msg->size, msg->uaddr, msg->perm)) { ret = -ENOMEM; break; @@ -1126,7 +1126,7 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, } vhost_vq_meta_reset(dev); vhost_del_umem_range(dev->iotlb, msg->iova, - msg->iova + msg->size - 1); + msg->iova + msg->size); break; default: ret = -EINVAL; @@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq, { const struct vhost_umem_node *node; struct vhost_umem *umem = vq->iotlb; - u64 s = 0, size, orig_addr = addr, last = addr + len - 1; + u64 s = 0, size, orig_addr = addr, last = addr + len; if (vhost_vq_meta_fetch(vq, addr, len, type)) return true; while (len > s) { node = vhost_umem_interval_tree_iter_first(&umem->umem_tree, - addr, - last); + addr, last); if (node == NULL || node->start > addr) { vhost_iotlb_miss(vq, addr, access); return false; @@ -1455,7 +1454,7 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) region->guest_phys_addr, region->memory_size, region->guest_phys_addr + - region->memory_size - 1, + region->memory_size, region->userspace_addr, VHOST_ACCESS_RW)) goto err; @@ -2055,7 +2054,7 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, } node = vhost_umem_interval_tree_iter_first(&umem->umem_tree, - addr, addr + len - 1); + addr, addr + len); if (node == NULL || node->start > addr) { if (umem != dev->iotlb) { ret = -EFAULT; diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index e9ed2722b633..bb36cb9ed5ec 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -53,13 +53,13 @@ struct vhost_log { }; #define START(node) ((node)->start) -#define LAST(node) ((node)->last) +#define END(node) ((node)->end) struct vhost_umem_node { struct rb_node rb; struct list_head link; __u64 start; - __u64 last; + __u64 end; __u64 size; __u64 userspace_addr; __u32 perm;
The vhost_umem interval tree really wants [a, b) intervals, not fully closed as currently. As such convert it to use the new interval_tree_gen.h, and also rename the 'last' endpoint in the node to 'end', which both a more suitable name for the half closed interval and also reduces the chances of some caller being missed. Cc: Michael S. Tsirkin" <mst@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: virtualization@lists.linux-foundation.org Signed-off-by: Davidlohr Bueso <dbueso@suse.de> --- drivers/vhost/vhost.c | 19 +++++++++---------- drivers/vhost/vhost.h | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-)