On Thu, Nov 11 2021, Yishai Hadas <yish...@nvidia.com> wrote: > Upon reading/writing the migration data there is no real reason to limit > the read/write system call from the file to be 8 bytes. > > In addition, there is no reason to depend on the file offset alignment. > The offset is just some logical value which depends also on the region > index and has nothing to do with the amount of data that can be > accessed. > > Move to read/write the full region size per chunk, this reduces > dramatically the number of the systems calls that are needed and improve > performance. > > Signed-off-by: Yishai Hadas <yish...@nvidia.com> > --- > hw/vfio/migration.c | 36 ++---------------------------------- > 1 file changed, 2 insertions(+), 34 deletions(-) > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index ff6b45de6b5..b5f310bb831 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -62,40 +62,8 @@ static inline int vfio_mig_access(VFIODevice *vbasedev, > void *val, int count, > return 0; > } > > -static int vfio_mig_rw(VFIODevice *vbasedev, __u8 *buf, size_t count, > - off_t off, bool iswrite) > -{ > - int ret, done = 0; > - __u8 *tbuf = buf; > - > - while (count) { > - int bytes = 0; > - > - if (count >= 8 && !(off % 8)) { > - bytes = 8; > - } else if (count >= 4 && !(off % 4)) { > - bytes = 4; > - } else if (count >= 2 && !(off % 2)) { > - bytes = 2; > - } else { > - bytes = 1; > - } > - > - ret = vfio_mig_access(vbasedev, tbuf, bytes, off, iswrite); > - if (ret) { > - return ret; > - } > - > - count -= bytes; > - done += bytes; > - off += bytes; > - tbuf += bytes; > - } > - return done; > -} > - > -#define vfio_mig_read(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o, > false) > -#define vfio_mig_write(f, v, c, o) vfio_mig_rw(f, (__u8 *)v, c, o, true) > +#define vfio_mig_read(f, v, c, o) vfio_mig_access(f, (__u8 *)v, c, o, > false) > +#define vfio_mig_write(f, v, c, o) vfio_mig_access(f, (__u8 *)v, c, o, > true) > > #define VFIO_MIG_STRUCT_OFFSET(f) \ > offsetof(struct vfio_device_migration_info, > f)
I've been looking at this patch and it doesn't look wrong to me in any obvious way. The question is: why had it been done like that in the first place? I dug through the old mailing list discussions, and it seems it had been introduced in v26, but I don't see any explanation for that. Kirti, do you remember why you added this?