Re: [PATCH v4 02/16] libs/guest: Reduce number of I/O vectors in write_batch

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Mon, 8 Jun 2026 at 15:43, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>
> On Wed, Jun 03, 2026 at 02:05:49PM +0100, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> >
> > Small optimization.
> > Reduce number of pieces passed to writev.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> > --
> > Changes since v2:
> > - change prefix in subject.
> > ---
> >  tools/libs/guest/xg_sr_save.c | 35 ++++++++++++++++-------------------
> >  1 file changed, 16 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
> > index 3b2c5222e4..1700d81905 100644
> > --- a/tools/libs/guest/xg_sr_save.c
> > +++ b/tools/libs/guest/xg_sr_save.c
> > @@ -97,9 +97,12 @@ static int write_batch(struct xc_sr_context *ctx)
> >      void *page, *orig_page;
> >      uint64_t *rec_pfns = NULL;
> >      struct iovec *iov = NULL; int iovcnt = 0;
> > -    struct xc_sr_rec_page_data_header hdr = { 0 };
> > -    struct xc_sr_record rec = {
> > -        .type = REC_TYPE_PAGE_DATA,
> > +    struct {
> > +        struct xc_sr_rhdr rec;
> > +        struct xc_sr_rec_page_data_header page_data;
>
> Is there a possible worry that the compiler (for another
> architecture) will introduce non-zero padding between those two structs?
>

No, the first structure is aligned to 32 bits (we don't support 16
bits systems), the second 32 or 64 bits.
In both cases the final structure won't need padding in the middle.

> > +    } hdrs = {
> > +        { .type = REC_TYPE_PAGE_DATA },
> > +        { 0 },
>
> Do you need the explicit initialization to 0 here?  All unspecified
> fields in the initialization will already be set to 0.
>

I'll remove it.

> >      };
> >
> >      assert(nr_pfns != 0);
> > @@ -115,7 +118,7 @@ static int write_batch(struct xc_sr_context *ctx)
> >      /* Pointers to locally allocated pages.  Need freeing. */
> >      local_pages = calloc(nr_pfns, sizeof(*local_pages));
> >      /* iovec[] for writev(). */
> > -    iov = malloc((nr_pfns + 4) * sizeof(*iov));
> > +    iov = malloc((nr_pfns + 2) * sizeof(*iov));
>
> It would seem more natural to use calloc() here, but it would also do
> a zeroing that we don't care.
>
> Thanks, Roger.

Frediano