Re: [PATCH v2 2/3] virtio_ring: Avoid using inline for small functions
On 2023-03-16 p.m.11:16, Jason Wang wrote:
External email: Use caution opening links or attachments
On Thu, Mar 16, 2023 at 2:55 AM Feng Liu wrote:
Remove the inline keyword, according to kernel coding style [1], defining
inline functions is not necessary for samll functions.
It is verified with GCC 12.2.0, the generated code with/without inline
is the same. Additionally tested with kernel pktgen and iperf, and
verified the result, pps of the results are the same in the cases of
with/without inline.
[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
Signed-off-by: Feng Liu
Reviewed-by: Jiri Pirko
Acked-by: Jason Wang
Thanks
Hi Michael & Jason
Could you please help to take these reviewed/acked patches forward?
Thanks so much
---
drivers/virtio/virtio_ring.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..a26fab91c59f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
-static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
- unsigned int total_sg)
+static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+ unsigned int total_sg)
{
/*
* If the host supports indirect descriptor tables, and we have
multiple
@@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev,
size_t size,
* making all of the arch DMA ops work on the vring device itself
* is a mess.
*/
-static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
+static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
{
return vq->dma_dev;
}
@@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
}
}
-static inline bool more_used_split(const struct vring_virtqueue *vq)
+static bool more_used_split(const struct vring_virtqueue *vq)
{
return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
vq->split.vring.used->idx);
@@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue
*_vq, u32 num)
/*
* Packed ring specific functions - *_packed().
*/
-static inline bool packed_used_wrap_counter(u16 last_used_idx)
+static bool packed_used_wrap_counter(u16 last_used_idx)
{
return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
-static inline u16 packed_last_used(u16 last_used_idx)
+static u16 packed_last_used(u16 last_used_idx)
{
return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
@@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct
vring_virtqueue *vq,
return avail == used && used == used_wrap_counter;
}
-static inline bool more_used_packed(const struct vring_virtqueue *vq)
+static bool more_used_packed(const struct vring_virtqueue *vq)
{
u16 last_used;
u16 last_used_idx;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/3] virtio_ring: Avoid using inline for small functions
Feng Liu writes:
> Remove the inline keyword, according to kernel coding style [1], defining
> inline functions is not necessary for samll functions.
>
> It is verified with GCC 12.2.0, the generated code with/without inline
> is the same. Additionally tested with kernel pktgen and iperf, and
> verified the result, pps of the results are the same in the cases of
> with/without inline.
>
> [1]
> https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
>
> Signed-off-by: Feng Liu
> Reviewed-by: Jiri Pirko
Reviewed-by: David Edmondson
> ---
> drivers/virtio/virtio_ring.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 41144b5246a8..a26fab91c59f 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
>
> #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
>
> -static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> - unsigned int total_sg)
> +static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> +unsigned int total_sg)
> {
> /*
>* If the host supports indirect descriptor tables, and we have multiple
> @@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev,
> size_t size,
> * making all of the arch DMA ops work on the vring device itself
> * is a mess.
> */
> -static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> +static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> {
> return vq->dma_dev;
> }
> @@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq,
> unsigned int head,
> }
> }
>
> -static inline bool more_used_split(const struct vring_virtqueue *vq)
> +static bool more_used_split(const struct vring_virtqueue *vq)
> {
> return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
> vq->split.vring.used->idx);
> @@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue
> *_vq, u32 num)
> /*
> * Packed ring specific functions - *_packed().
> */
> -static inline bool packed_used_wrap_counter(u16 last_used_idx)
> +static bool packed_used_wrap_counter(u16 last_used_idx)
> {
> return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
> }
>
> -static inline u16 packed_last_used(u16 last_used_idx)
> +static u16 packed_last_used(u16 last_used_idx)
> {
> return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
> }
> @@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct
> vring_virtqueue *vq,
> return avail == used && used == used_wrap_counter;
> }
>
> -static inline bool more_used_packed(const struct vring_virtqueue *vq)
> +static bool more_used_packed(const struct vring_virtqueue *vq)
> {
> u16 last_used;
> u16 last_used_idx;
> --
> 2.34.1
--
But are you safe Miss Gradenko?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/3] virtio_ring: Avoid using inline for small functions
On Thu, Mar 16, 2023 at 2:55 AM Feng Liu wrote:
>
> Remove the inline keyword, according to kernel coding style [1], defining
> inline functions is not necessary for samll functions.
>
> It is verified with GCC 12.2.0, the generated code with/without inline
> is the same. Additionally tested with kernel pktgen and iperf, and
> verified the result, pps of the results are the same in the cases of
> with/without inline.
>
> [1]
> https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
>
> Signed-off-by: Feng Liu
> Reviewed-by: Jiri Pirko
Acked-by: Jason Wang
Thanks
> ---
> drivers/virtio/virtio_ring.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 41144b5246a8..a26fab91c59f 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
>
> #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
>
> -static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> - unsigned int total_sg)
> +static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
> + unsigned int total_sg)
> {
> /*
> * If the host supports indirect descriptor tables, and we have
> multiple
> @@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev,
> size_t size,
> * making all of the arch DMA ops work on the vring device itself
> * is a mess.
> */
> -static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> +static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> {
> return vq->dma_dev;
> }
> @@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq,
> unsigned int head,
> }
> }
>
> -static inline bool more_used_split(const struct vring_virtqueue *vq)
> +static bool more_used_split(const struct vring_virtqueue *vq)
> {
> return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
> vq->split.vring.used->idx);
> @@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue
> *_vq, u32 num)
> /*
> * Packed ring specific functions - *_packed().
> */
> -static inline bool packed_used_wrap_counter(u16 last_used_idx)
> +static bool packed_used_wrap_counter(u16 last_used_idx)
> {
> return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
> }
>
> -static inline u16 packed_last_used(u16 last_used_idx)
> +static u16 packed_last_used(u16 last_used_idx)
> {
> return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
> }
> @@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct
> vring_virtqueue *vq,
> return avail == used && used == used_wrap_counter;
> }
>
> -static inline bool more_used_packed(const struct vring_virtqueue *vq)
> +static bool more_used_packed(const struct vring_virtqueue *vq)
> {
> u16 last_used;
> u16 last_used_idx;
> --
> 2.34.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/3] virtio_ring: Avoid using inline for small functions
Remove the inline keyword, according to kernel coding style [1], defining
inline functions is not necessary for samll functions.
It is verified with GCC 12.2.0, the generated code with/without inline
is the same. Additionally tested with kernel pktgen and iperf, and
verified the result, pps of the results are the same in the cases of
with/without inline.
[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
Signed-off-by: Feng Liu
Reviewed-by: Jiri Pirko
---
drivers/virtio/virtio_ring.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..a26fab91c59f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
-static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
- unsigned int total_sg)
+static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+ unsigned int total_sg)
{
/*
* If the host supports indirect descriptor tables, and we have multiple
@@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev,
size_t size,
* making all of the arch DMA ops work on the vring device itself
* is a mess.
*/
-static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
+static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
{
return vq->dma_dev;
}
@@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
}
}
-static inline bool more_used_split(const struct vring_virtqueue *vq)
+static bool more_used_split(const struct vring_virtqueue *vq)
{
return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
vq->split.vring.used->idx);
@@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue
*_vq, u32 num)
/*
* Packed ring specific functions - *_packed().
*/
-static inline bool packed_used_wrap_counter(u16 last_used_idx)
+static bool packed_used_wrap_counter(u16 last_used_idx)
{
return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
-static inline u16 packed_last_used(u16 last_used_idx)
+static u16 packed_last_used(u16 last_used_idx)
{
return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
@@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct
vring_virtqueue *vq,
return avail == used && used == used_wrap_counter;
}
-static inline bool more_used_packed(const struct vring_virtqueue *vq)
+static bool more_used_packed(const struct vring_virtqueue *vq)
{
u16 last_used;
u16 last_used_idx;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/3] virtio_ring: Avoid using inline for small functions
According to kernel coding style [1], defining inline functions is not
necessary and beneficial for simple functions. Hence clean up the code
by removing the inline keyword.
It is verified with GCC 12.2.0, the generated code with/without inline
is same. Additionally tested with pktgen and iperf, and verified the
result, the pps test results are the same in the cases of with/without
inline.
Iperf and pps of pktgen for virtio-net didn't change before and after
the change.
[1]
https://www.kernel.org/doc/html/v6.2-rc3/process/coding-style.html#the-inline-disease
Signed-off-by: Feng Liu
Reviewed-by: Jiri Pirko
Reviewed-by: Parav Pandit
Reviewed-by: Gavin Li
Reviewed-by: Bodong Wang
Reviewed-by: David Edmondson
---
drivers/virtio/virtio_ring.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..a26fab91c59f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -233,8 +233,8 @@ static void vring_free(struct virtqueue *_vq);
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
-static inline bool virtqueue_use_indirect(struct vring_virtqueue *vq,
- unsigned int total_sg)
+static bool virtqueue_use_indirect(struct vring_virtqueue *vq,
+ unsigned int total_sg)
{
/*
* If the host supports indirect descriptor tables, and we have multiple
@@ -349,7 +349,7 @@ static void vring_free_queue(struct virtio_device *vdev,
size_t size,
* making all of the arch DMA ops work on the vring device itself
* is a mess.
*/
-static inline struct device *vring_dma_dev(const struct vring_virtqueue *vq)
+static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
{
return vq->dma_dev;
}
@@ -784,7 +784,7 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
}
}
-static inline bool more_used_split(const struct vring_virtqueue *vq)
+static bool more_used_split(const struct vring_virtqueue *vq)
{
return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
vq->split.vring.used->idx);
@@ -1172,12 +1172,12 @@ static int virtqueue_resize_split(struct virtqueue
*_vq, u32 num)
/*
* Packed ring specific functions - *_packed().
*/
-static inline bool packed_used_wrap_counter(u16 last_used_idx)
+static bool packed_used_wrap_counter(u16 last_used_idx)
{
return !!(last_used_idx & (1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
-static inline u16 packed_last_used(u16 last_used_idx)
+static u16 packed_last_used(u16 last_used_idx)
{
return last_used_idx & ~(-(1 << VRING_PACKED_EVENT_F_WRAP_CTR));
}
@@ -1612,7 +1612,7 @@ static inline bool is_used_desc_packed(const struct
vring_virtqueue *vq,
return avail == used && used == used_wrap_counter;
}
-static inline bool more_used_packed(const struct vring_virtqueue *vq)
+static bool more_used_packed(const struct vring_virtqueue *vq)
{
u16 last_used;
u16 last_used_idx;
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
