Feng Liu
2023-Mar-30 18:22 UTC
[PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params
On 2023-03-16 p.m.11:17, Jason Wang wrote:> External email: Use caution opening links or attachments > > > On Thu, Mar 16, 2023 at 2:55?AM Feng Liu <feliu at nvidia.com> wrote: >> >> Add const to make the read-only pointer parameters clear, similar to >> many existing functions. >> >> Use `container_of_const` to implement `to_vvq`, which ensures the >> const-ness of read-only parameters and avoids accidental modification >> of their members. >> >> Signed-off-by: Feng Liu <feliu at nvidia.com> >> Reviewed-by: Jiri Pirko <jiri at nvidia.com> > > Acked-by: Jason Wang <jasowang at redhat.com> > > Thanks >Hi Michael & Jason Could you please help to take these reviewed/acked patches forward? Thanks so much>> >> --- >> v0 -> v1 >> feedbacks from Michael S. Tsirkin >> - use `container_of_const` to implement `to_vvq` >> --- >> drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------ >> include/linux/virtio.h | 14 +++++++------- >> 2 files changed, 25 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >> index a26fab91c59f..4c3bb0ddeb9b 100644 >> --- a/drivers/virtio/virtio_ring.c >> +++ b/drivers/virtio/virtio_ring.c >> @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq); >> * Helpers. >> */ >> >> -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) >> +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq) >> >> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq, >> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq, >> unsigned int total_sg) >> { >> /* >> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq, >> * unconditionally on data path. >> */ >> >> -static bool vring_use_dma_api(struct virtio_device *vdev) >> +static bool vring_use_dma_api(const struct virtio_device *vdev) >> { >> if (!virtio_has_dma_quirk(vdev)) >> return true; >> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev) >> return false; >> } >> >> -size_t virtio_max_dma_size(struct virtio_device *vdev) >> +size_t virtio_max_dma_size(const struct virtio_device *vdev) >> { >> size_t max_segment_size = SIZE_MAX; >> >> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num) >> */ >> >> static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq, >> - struct vring_desc *desc) >> + const struct vring_desc *desc) >> { >> u16 flags; >> >> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx) >> } >> >> static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, >> - struct vring_desc_extra *extra) >> + const struct vring_desc_extra *extra) >> { >> u16 flags; >> >> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, >> } >> >> static void vring_unmap_desc_packed(const struct vring_virtqueue *vq, >> - struct vring_packed_desc *desc) >> + const struct vring_packed_desc *desc) >> { >> u16 flags; >> >> @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features); >> * Returns the size of the vring. This is mainly used for boasting to >> * userspace. Unlike other operations, this need not be serialized. >> */ >> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) >> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq) >> { >> >> - struct vring_virtqueue *vq = to_vvq(_vq); >> + const struct vring_virtqueue *vq = to_vvq(_vq); >> >> return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num; >> } >> @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq) >> } >> EXPORT_SYMBOL_GPL(__virtqueue_unbreak); >> >> -bool virtqueue_is_broken(struct virtqueue *_vq) >> +bool virtqueue_is_broken(const struct virtqueue *_vq) >> { >> - struct vring_virtqueue *vq = to_vvq(_vq); >> + const struct vring_virtqueue *vq = to_vvq(_vq); >> >> return READ_ONCE(vq->broken); >> } >> @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev) >> } >> EXPORT_SYMBOL_GPL(__virtio_unbreak_device); >> >> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) >> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq) >> { >> - struct vring_virtqueue *vq = to_vvq(_vq); >> + const struct vring_virtqueue *vq = to_vvq(_vq); >> >> BUG_ON(!vq->we_own_ring); >> >> @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) >> } >> EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr); >> >> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) >> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq) >> { >> - struct vring_virtqueue *vq = to_vvq(_vq); >> + const struct vring_virtqueue *vq = to_vvq(_vq); >> >> BUG_ON(!vq->we_own_ring); >> >> @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) >> } >> EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr); >> >> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) >> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq) >> { >> - struct vring_virtqueue *vq = to_vvq(_vq); >> + const struct vring_virtqueue *vq = to_vvq(_vq); >> >> BUG_ON(!vq->we_own_ring); >> >> @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) >> EXPORT_SYMBOL_GPL(virtqueue_get_used_addr); >> >> /* Only available for split ring */ >> -const struct vring *virtqueue_get_vring(struct virtqueue *vq) >> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq) >> { >> return &to_vvq(vq)->split.vring; >> } >> diff --git a/include/linux/virtio.h b/include/linux/virtio.h >> index 2b472514c49b..c4225653f949 100644 >> --- a/include/linux/virtio.h >> +++ b/include/linux/virtio.h >> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq); >> >> void *virtqueue_detach_unused_buf(struct virtqueue *vq); >> >> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq); >> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq); >> >> -bool virtqueue_is_broken(struct virtqueue *vq); >> +bool virtqueue_is_broken(const struct virtqueue *vq); >> >> -const struct vring *virtqueue_get_vring(struct virtqueue *vq); >> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq); >> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq); >> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq); >> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq); >> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq); >> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq); >> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq); >> >> int virtqueue_resize(struct virtqueue *vq, u32 num, >> void (*recycle)(struct virtqueue *vq, void *buf)); >> @@ -147,7 +147,7 @@ int virtio_device_restore(struct virtio_device *dev); >> #endif >> void virtio_reset_device(struct virtio_device *dev); >> >> -size_t virtio_max_dma_size(struct virtio_device *vdev); >> +size_t virtio_max_dma_size(const struct virtio_device *vdev); >> >> #define virtio_device_for_each_vq(vdev, vq) \ >> list_for_each_entry(vq, &vdev->vqs, list) >> -- >> 2.34.1 >> >
Michael S. Tsirkin
2023-Mar-30 20:27 UTC
[PATCH v2 3/3] virtio_ring: Use const to annotate read-only pointer params
On Thu, Mar 30, 2023 at 02:22:44PM -0400, Feng Liu wrote:> > > On 2023-03-16 p.m.11:17, Jason Wang wrote: > > External email: Use caution opening links or attachments > > > > > > On Thu, Mar 16, 2023 at 2:55?AM Feng Liu <feliu at nvidia.com> wrote: > > > > > > Add const to make the read-only pointer parameters clear, similar to > > > many existing functions. > > > > > > Use `container_of_const` to implement `to_vvq`, which ensures the > > > const-ness of read-only parameters and avoids accidental modification > > > of their members. > > > > > > Signed-off-by: Feng Liu <feliu at nvidia.com> > > > Reviewed-by: Jiri Pirko <jiri at nvidia.com> > > > > Acked-by: Jason Wang <jasowang at redhat.com> > > > > Thanks > > > Hi Michael & Jason > Could you please help to take these reviewed/acked patches forward? > Thanks so muchthis is not going in this linux, only the next.> > > > > > > --- > > > v0 -> v1 > > > feedbacks from Michael S. Tsirkin > > > - use `container_of_const` to implement `to_vvq` > > > --- > > > drivers/virtio/virtio_ring.c | 36 ++++++++++++++++++------------------ > > > include/linux/virtio.h | 14 +++++++------- > > > 2 files changed, 25 insertions(+), 25 deletions(-) > > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > > index a26fab91c59f..4c3bb0ddeb9b 100644 > > > --- a/drivers/virtio/virtio_ring.c > > > +++ b/drivers/virtio/virtio_ring.c > > > @@ -231,9 +231,9 @@ static void vring_free(struct virtqueue *_vq); > > > * Helpers. > > > */ > > > > > > -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) > > > +#define to_vvq(_vq) container_of_const(_vq, struct vring_virtqueue, vq) > > > > > > -static bool virtqueue_use_indirect(struct vring_virtqueue *vq, > > > +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq, > > > unsigned int total_sg) > > > { > > > /* > > > @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq, > > > * unconditionally on data path. > > > */ > > > > > > -static bool vring_use_dma_api(struct virtio_device *vdev) > > > +static bool vring_use_dma_api(const struct virtio_device *vdev) > > > { > > > if (!virtio_has_dma_quirk(vdev)) > > > return true; > > > @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev) > > > return false; > > > } > > > > > > -size_t virtio_max_dma_size(struct virtio_device *vdev) > > > +size_t virtio_max_dma_size(const struct virtio_device *vdev) > > > { > > > size_t max_segment_size = SIZE_MAX; > > > > > > @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num) > > > */ > > > > > > static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq, > > > - struct vring_desc *desc) > > > + const struct vring_desc *desc) > > > { > > > u16 flags; > > > > > > @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx) > > > } > > > > > > static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, > > > - struct vring_desc_extra *extra) > > > + const struct vring_desc_extra *extra) > > > { > > > u16 flags; > > > > > > @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, > > > } > > > > > > static void vring_unmap_desc_packed(const struct vring_virtqueue *vq, > > > - struct vring_packed_desc *desc) > > > + const struct vring_packed_desc *desc) > > > { > > > u16 flags; > > > > > > @@ -2786,10 +2786,10 @@ EXPORT_SYMBOL_GPL(vring_transport_features); > > > * Returns the size of the vring. This is mainly used for boasting to > > > * userspace. Unlike other operations, this need not be serialized. > > > */ > > > -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) > > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq) > > > { > > > > > > - struct vring_virtqueue *vq = to_vvq(_vq); > > > + const struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > return vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num; > > > } > > > @@ -2819,9 +2819,9 @@ void __virtqueue_unbreak(struct virtqueue *_vq) > > > } > > > EXPORT_SYMBOL_GPL(__virtqueue_unbreak); > > > > > > -bool virtqueue_is_broken(struct virtqueue *_vq) > > > +bool virtqueue_is_broken(const struct virtqueue *_vq) > > > { > > > - struct vring_virtqueue *vq = to_vvq(_vq); > > > + const struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > return READ_ONCE(vq->broken); > > > } > > > @@ -2868,9 +2868,9 @@ void __virtio_unbreak_device(struct virtio_device *dev) > > > } > > > EXPORT_SYMBOL_GPL(__virtio_unbreak_device); > > > > > > -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) > > > +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *_vq) > > > { > > > - struct vring_virtqueue *vq = to_vvq(_vq); > > > + const struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > BUG_ON(!vq->we_own_ring); > > > > > > @@ -2881,9 +2881,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) > > > } > > > EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr); > > > > > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) > > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq) > > > { > > > - struct vring_virtqueue *vq = to_vvq(_vq); > > > + const struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > BUG_ON(!vq->we_own_ring); > > > > > > @@ -2895,9 +2895,9 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) > > > } > > > EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr); > > > > > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) > > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq) > > > { > > > - struct vring_virtqueue *vq = to_vvq(_vq); > > > + const struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > BUG_ON(!vq->we_own_ring); > > > > > > @@ -2910,7 +2910,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) > > > EXPORT_SYMBOL_GPL(virtqueue_get_used_addr); > > > > > > /* Only available for split ring */ > > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq) > > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq) > > > { > > > return &to_vvq(vq)->split.vring; > > > } > > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > > > index 2b472514c49b..c4225653f949 100644 > > > --- a/include/linux/virtio.h > > > +++ b/include/linux/virtio.h > > > @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq); > > > > > > void *virtqueue_detach_unused_buf(struct virtqueue *vq); > > > > > > -unsigned int virtqueue_get_vring_size(struct virtqueue *vq); > > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq); > > > > > > -bool virtqueue_is_broken(struct virtqueue *vq); > > > +bool virtqueue_is_broken(const struct virtqueue *vq); > > > > > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq); > > > -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq); > > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq); > > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq); > > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq); > > > +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq); > > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq); > > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq); > > > > > > int virtqueue_resize(struct virtqueue *vq, u32 num, > > > void (*recycle)(struct virtqueue *vq, void *buf)); > > > @@ -147,7 +147,7 @@ int virtio_device_restore(struct virtio_device *dev); > > > #endif > > > void virtio_reset_device(struct virtio_device *dev); > > > > > > -size_t virtio_max_dma_size(struct virtio_device *vdev); > > > +size_t virtio_max_dma_size(const struct virtio_device *vdev); > > > > > > #define virtio_device_for_each_vq(vdev, vq) \ > > > list_for_each_entry(vq, &vdev->vqs, list) > > > -- > > > 2.34.1 > > > > >