On Mon, Aug 26, 2019 at 4:46 PM Eric Blake <ebl...@redhat.com> wrote: > > On 8/25/19 5:03 PM, Nir Soffer wrote: > > When creating an image with preallocation "off" or "falloc", the first > > block of the image is typically not allocated. When using Gluster > > storage backed by XFS filesystem, reading this block using direct I/O > > succeeds regardless of request length, fooling alignment detection. > > > > In this case we fallback to a safe value (4096) instead of the optimal > > value (512), which may lead to unneeded data copying when aligning > > requests. Allocating the first block avoids the fallback. > > > > > Here is a table comparing the total time spent: > > > > Type Before(s) After(s) Diff(%) > > --------------------------------------- > > real 530.028 469.123 -11.4 > > user 17.204 10.768 -37.4 > > sys 17.881 7.011 -60.7 > > > > We can see very clear improvement in CPU usage. > > Nice justification. > > > > +/* > > + * Help alignment probing by allocating the first block. > > + * > > > + do { > > + n = pwrite(fd, buf, write_size, 0); > > + } while (n == -1 && errno == EINTR); > > + > > + qemu_vfree(buf); > > qemu_vfree() can corrupt errno... > > > + > > + return (n == -1) ? -errno : 0; > > ...which means you may be returning an unexpected value here. > > Either we should patch qemu_vfree() to guarantee that errno is > preserved, or you locally capture errno before calling it here.
qemu_vfree() returns void like free(), so changing errno is unexpected, but other code using it should not be effected, so preserving errno here seems like a better change. Thanks! Nir