Re: [Libguestfs] [nbdkit PATCH] data, memory: Optimize .zero > PAGE_SIZE
On 8/15/19 5:25 AM, Richard W.M. Jones wrote: > On Wed, Aug 14, 2019 at 09:10:15PM -0500, Eric Blake wrote: >> When sparse_array_zero() is used for a range larger than a page, >> there's no need to waste time in memset() or is_zero() - we already >> know the page will be free()d. >> >> Signed-off-by: Eric Blake >> --- >> >> Here's a fun one :) >> >> common/sparse/sparse.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c >> index cb44743c..5e085763 100644 >> --- a/common/sparse/sparse.c >> +++ b/common/sparse/sparse.c >> @@ -343,10 +343,13 @@ sparse_array_zero (struct sparse_array *sa, uint32_t >> count, uint64_t offset) >>n = count; >> >> if (p) { >> - memset (p, 0, n); >> + if (n < PAGE_SIZE) >> +memset (p, 0, n); >> + else >> +assert (p == *l2_page); >> >>/* If the whole page is now zero, free it. */ >> - if (is_zero (*l2_page, PAGE_SIZE)) { >> + if (n == PAGE_SIZE || is_zero (*l2_page, PAGE_SIZE)) { > > Should this be n >= PAGE_SIZE? My guess is no because lookup (..&n..) > can't return n larger than the remaining size of the page > (ie. n <= PAGE_SIZE). Correct, but that's action at a distance; using >= doesn't change semantics and brings the action closer to the code. I'll make that tweak. > >> if (sa->debug) >>nbdkit_debug ("%s: freeing zero page at offset %" PRIu64, >> __func__, offset); > > Anyway, ACK. > > Rich. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit PATCH] data, memory: Optimize .zero > PAGE_SIZE
On Wed, Aug 14, 2019 at 09:10:15PM -0500, Eric Blake wrote: > When sparse_array_zero() is used for a range larger than a page, > there's no need to waste time in memset() or is_zero() - we already > know the page will be free()d. > > Signed-off-by: Eric Blake > --- > > Here's a fun one :) > > common/sparse/sparse.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/sparse/sparse.c b/common/sparse/sparse.c > index cb44743c..5e085763 100644 > --- a/common/sparse/sparse.c > +++ b/common/sparse/sparse.c > @@ -343,10 +343,13 @@ sparse_array_zero (struct sparse_array *sa, uint32_t > count, uint64_t offset) >n = count; > > if (p) { > - memset (p, 0, n); > + if (n < PAGE_SIZE) > +memset (p, 0, n); > + else > +assert (p == *l2_page); > >/* If the whole page is now zero, free it. */ > - if (is_zero (*l2_page, PAGE_SIZE)) { > + if (n == PAGE_SIZE || is_zero (*l2_page, PAGE_SIZE)) { Should this be n >= PAGE_SIZE? My guess is no because lookup (..&n..) can't return n larger than the remaining size of the page (ie. n <= PAGE_SIZE). > if (sa->debug) >nbdkit_debug ("%s: freeing zero page at offset %" PRIu64, > __func__, offset); Anyway, ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs