David Edmondson
2023-Mar-07 09:14 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
Feng Liu via Virtualization <virtualization at lists.linux-foundation.org> writes:> Add const to make the read-only pointer parameters clear, similar to > many existing functions.In many of the modified functions the local variable that is a cast of the argument could also be const. Is there a reason not to do both at the same time?> Signed-off-by: Feng Liu <feliu at nvidia.com> > Reviewed-by: Jiri Pirko <jiri at nvidia.com> > Reviewed-by: Parav Pandit <parav at nvidia.com> > Reviewed-by: Gavin Li <gavinl at nvidia.com> > Reviewed-by: Bodong Wang <bodong at nvidia.com> > --- > drivers/virtio/virtio_ring.c | 25 ++++++++++++------------- > include/linux/virtio.h | 12 ++++++------ > 2 files changed, 18 insertions(+), 19 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 806cc44eae64..8010fd1d2047 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -233,7 +233,7 @@ static void vring_free(struct virtqueue *_vq); > > #define to_vvq(_vq) container_of(_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,7 +2786,7 @@ 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); > @@ -2819,7 +2819,7 @@ 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); > > @@ -2827,8 +2827,7 @@ bool virtqueue_is_broken(struct virtqueue *_vq) > } > EXPORT_SYMBOL_GPL(virtqueue_is_broken); > > -/* > - * This should prevent the device from being used, allowing drivers to > +/ This should prevent the device from being used, allowing drivers to > * recover. You may need to grab appropriate locks to flush. > */ > void virtio_break_device(struct virtio_device *dev) > @@ -2881,7 +2880,7 @@ 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); > > @@ -2895,7 +2894,7 @@ 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); > > @@ -2910,7 +2909,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..36a79374e735 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)); > -- > 2.34.1 > > _______________________________________________ > Virtualization mailing list > Virtualization at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization-- Woke up in my clothes again this morning, don't know exactly where I am.
Feng Liu
2023-Mar-07 21:17 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
On 2023-03-07 04:14, David Edmondson wrote:> External email: Use caution opening links or attachments > > > Feng Liu via Virtualization <virtualization at lists.linux-foundation.org> > writes: > >> Add const to make the read-only pointer parameters clear, similar to >> many existing functions. > > In many of the modified functions the local variable that is a cast of > the argument could also be const. Is there a reason not to do both at > the same time? >Hi,David In order to prevent the content of a pointer parameter from being modified and increase the readability of the function, it is recommended to add the 'const' keyword to the parameter. This is not necessary for local variables and non-pointer parameters, as they are only stored on the stack and do not affect the original value or structure member passed into the function. Therefore, in this case, the 'const' keyword is only added to pointer parameters.>> Signed-off-by: Feng Liu <feliu at nvidia.com> >> Reviewed-by: Jiri Pirko <jiri at nvidia.com> >> Reviewed-by: Parav Pandit <parav at nvidia.com> >> Reviewed-by: Gavin Li <gavinl at nvidia.com> >> Reviewed-by: Bodong Wang <bodong at nvidia.com> >> --- >> drivers/virtio/virtio_ring.c | 25 ++++++++++++------------- >> include/linux/virtio.h | 12 ++++++------ >> 2 files changed, 18 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >> index 806cc44eae64..8010fd1d2047 100644 >> --- a/drivers/virtio/virtio_ring.c >> +++ b/drivers/virtio/virtio_ring.c >> @@ -233,7 +233,7 @@ static void vring_free(struct virtqueue *_vq); >> >> #define to_vvq(_vq) container_of(_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,7 +2786,7 @@ 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); >> @@ -2819,7 +2819,7 @@ 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); >> >> @@ -2827,8 +2827,7 @@ bool virtqueue_is_broken(struct virtqueue *_vq) >> } >> EXPORT_SYMBOL_GPL(virtqueue_is_broken); >> >> -/* >> - * This should prevent the device from being used, allowing drivers to >> +/ This should prevent the device from being used, allowing drivers to >> * recover. You may need to grab appropriate locks to flush. >> */ >> void virtio_break_device(struct virtio_device *dev) >> @@ -2881,7 +2880,7 @@ 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); >> >> @@ -2895,7 +2894,7 @@ 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); >> >> @@ -2910,7 +2909,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..36a79374e735 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)); >> -- >> 2.34.1 >> >> _______________________________________________ >> Virtualization mailing list >> Virtualization at lists.linux-foundation.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linuxfoundation.org%2Fmailman%2Flistinfo%2Fvirtualization&data=05%7C01%7Cfeliu%40nvidia.com%7C6265cc6ac03e484167ba08db1eec5428%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638137772595544505%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=74eHEHUMF89nTG6hrrdMpkl43DJWvC9xCjwqNOny%2FQE%3D&reserved=0 > -- > Woke up in my clothes again this morning, don't know exactly where I am.