[PATCH v2] xen/public: add new macro to ring.h

Juergen Gross posted 1 patch 2 years ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220427070309.15090-1-jgross@suse.com
xen/include/public/io/ring.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[PATCH v2] xen/public: add new macro to ring.h
Posted by Juergen Gross 2 years ago
For the initialization of a ring page by the frontend two macros are
available in ring.h: SHARED_RING_INIT() and FRONT_RING_INIT().

All known users use always both of them in direct sequence.

Add another macro XEN_FRONT_RING_INIT() combining the two macros.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- modify comment (Jan Beulich)
- drop underscores from macro parameters (Jan Beulich)
---
 xen/include/public/io/ring.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h
index 277af36e61..ab3439bd58 100644
--- a/xen/include/public/io/ring.h
+++ b/xen/include/public/io/ring.h
@@ -95,9 +95,8 @@ typedef unsigned int RING_IDX;
  * of the shared memory area (PAGE_SIZE, for instance). To initialise
  * the front half:
  *
- *     mytag_front_ring_t front_ring;
- *     SHARED_RING_INIT((mytag_sring_t *)shared_page);
- *     FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
+ *     mytag_front_ring_t ring;
+ *     XEN_FRONT_RING_INIT(&ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
  *
  * Initializing the back follows similarly (note that only the front
  * initializes the shared ring):
@@ -184,6 +183,11 @@ typedef struct __name##_back_ring __name##_back_ring_t
 
 #define FRONT_RING_INIT(_r, _s, __size) FRONT_RING_ATTACH(_r, _s, 0, __size)
 
+#define XEN_FRONT_RING_INIT(r, s, size) do {                            \
+    SHARED_RING_INIT(s);                                                \
+    FRONT_RING_INIT(r, s, size);                                        \
+} while (0)
+
 #define BACK_RING_ATTACH(_r, _s, _i, __size) do {                       \
     (_r)->rsp_prod_pvt = (_i);                                          \
     (_r)->req_cons = (_i);                                              \
-- 
2.34.1
Re: [PATCH v2] xen/public: add new macro to ring.h
Posted by Jan Beulich 2 years ago
On 27.04.2022 09:03, Juergen Gross wrote:
> For the initialization of a ring page by the frontend two macros are
> available in ring.h: SHARED_RING_INIT() and FRONT_RING_INIT().
> 
> All known users use always both of them in direct sequence.
> 
> Add another macro XEN_FRONT_RING_INIT() combining the two macros.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit ...

> --- a/xen/include/public/io/ring.h
> +++ b/xen/include/public/io/ring.h
> @@ -95,9 +95,8 @@ typedef unsigned int RING_IDX;
>   * of the shared memory area (PAGE_SIZE, for instance). To initialise
>   * the front half:
>   *
> - *     mytag_front_ring_t front_ring;
> - *     SHARED_RING_INIT((mytag_sring_t *)shared_page);
> - *     FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
> + *     mytag_front_ring_t ring;
> + *     XEN_FRONT_RING_INIT(&ring, (mytag_sring_t *)shared_page, PAGE_SIZE);

... I had hoped for the original text to remain and the new macro
being suggested as a shorthand. But you're the maintainer ...

Jan
Re: [PATCH v2] xen/public: add new macro to ring.h
Posted by Juergen Gross 2 years ago
On 27.04.22 09:11, Jan Beulich wrote:
> On 27.04.2022 09:03, Juergen Gross wrote:
>> For the initialization of a ring page by the frontend two macros are
>> available in ring.h: SHARED_RING_INIT() and FRONT_RING_INIT().
>>
>> All known users use always both of them in direct sequence.
>>
>> Add another macro XEN_FRONT_RING_INIT() combining the two macros.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
> 
>> --- a/xen/include/public/io/ring.h
>> +++ b/xen/include/public/io/ring.h
>> @@ -95,9 +95,8 @@ typedef unsigned int RING_IDX;
>>    * of the shared memory area (PAGE_SIZE, for instance). To initialise
>>    * the front half:
>>    *
>> - *     mytag_front_ring_t front_ring;
>> - *     SHARED_RING_INIT((mytag_sring_t *)shared_page);
>> - *     FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
>> + *     mytag_front_ring_t ring;
>> + *     XEN_FRONT_RING_INIT(&ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
> 
> ... I had hoped for the original text to remain and the new macro
> being suggested as a shorthand. But you're the maintainer ...

I prefer the single macro, as it is less code and has the proper
name space.


Juergen