Re: [Libguestfs] [nbdkit PATCH] data, memory: Optimize .zero > PAGE_SIZE

[Eric Blake]

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 (..)
> 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

[Richard W.M. Jones]

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 (..)
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

[Libguestfs] [nbdkit PATCH] data, memory: Optimize .zero > PAGE_SIZE

[Eric Blake]

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)) {
 if (sa->debug)
   nbdkit_debug ("%s: freeing zero page at offset %" PRIu64,
 __func__, offset);
-- 
2.20.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs