[Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes

Leonid Bloch posted 9 patches 7 years, 4 months ago
[Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
Posted by Leonid Bloch 7 years, 4 months ago
The default cache-clean-interval is set to 10 minutes, in order to lower
the overhead of the qcow2 caches (before the default was 0, i.e.
disabled).

* For non-Linux platforms the default is kept at 0, because
  cache-clean-interval is not supported there yet.

Signed-off-by: Leonid Bloch <lbloch@janustech.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c        | 2 +-
 block/qcow2.h        | 4 +++-
 docs/qcow2-cache.txt | 4 ++--
 qapi/block-core.json | 3 ++-
 qemu-options.hx      | 2 +-
 5 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 20b5093269..95e1c98daa 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -944,7 +944,7 @@ static int qcow2_update_options_prepare(BlockDriverState *bs,
     /* New interval for cache cleanup timer */
     r->cache_clean_interval =
         qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL,
-                            s->cache_clean_interval);
+                            DEFAULT_CACHE_CLEAN_INTERVAL);
 #ifndef CONFIG_LINUX
     if (r->cache_clean_interval != 0) {
         error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL
diff --git a/block/qcow2.h b/block/qcow2.h
index 0f0e3534bf..ba430316b9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -76,13 +76,15 @@
 
 #ifdef CONFIG_LINUX
 #define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
+#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
 #else
 #define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
+/* Cache clean interval is currently available only on Linux, so must be 0 */
+#define DEFAULT_CACHE_CLEAN_INTERVAL 0
 #endif
 
 #define DEFAULT_CLUSTER_SIZE S_64KiB
 
-
 #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
 #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request"
 #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot"
diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 1fcc0658b2..59358b816f 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -210,8 +210,8 @@ This example removes all unused cache entries every 15 minutes:
 
    -drive file=hd.qcow2,cache-clean-interval=900
 
-If unset, the default value for this parameter is 0 and it disables
-this feature.
+If unset, the default value for this parameter is 600. Setting it to 0
+disables this feature.
 
 Note that this functionality currently relies on the MADV_DONTNEED
 argument for madvise() to actually free the memory. This is a
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ac3b48ee54..46dac23d2f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2895,7 +2895,8 @@
 #
 # @cache-clean-interval:  clean unused entries in the L2 and refcount
 #                         caches. The interval is in seconds. The default value
-#                         is 0 and it disables this feature (since 2.5)
+#                         is 600, and 0 disables this feature. (since 2.5)
+#
 # @encrypt:               Image decryption options. Mandatory for
 #                         encrypted images, except when doing a metadata-only
 #                         probe of the image. (since 2.10)
diff --git a/qemu-options.hx b/qemu-options.hx
index 14aee78c6c..52d9d9f06d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -747,7 +747,7 @@ it which is not used for the L2 cache)
 
 @item cache-clean-interval
 Clean unused entries in the L2 and refcount caches. The interval is in seconds.
-The default value is 0 and it disables this feature.
+The default value is 600. Setting it to 0 disables this feature.
 
 @item pass-discard-request
 Whether discard requests to the qcow2 device should be forwarded to the data
-- 
2.17.1


Re: [Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
Posted by Eric Blake 7 years, 4 months ago
On 9/26/18 11:04 AM, Leonid Bloch wrote:
> The default cache-clean-interval is set to 10 minutes, in order to lower
> the overhead of the qcow2 caches (before the default was 0, i.e.
> disabled).
> 
> * For non-Linux platforms the default is kept at 0, because
>    cache-clean-interval is not supported there yet.
> 
> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---

> @@ -76,13 +76,15 @@
>   
>   #ifdef CONFIG_LINUX
>   #define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
> +#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
>   #else
>   #define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
> +/* Cache clean interval is currently available only on Linux, so must be 0 */
> +#define DEFAULT_CACHE_CLEAN_INTERVAL 0
>   #endif
>   

> +++ b/docs/qcow2-cache.txt
> @@ -210,8 +210,8 @@ This example removes all unused cache entries every 15 minutes:
>   
>      -drive file=hd.qcow2,cache-clean-interval=900
>   
> -If unset, the default value for this parameter is 0 and it disables
> -this feature.
> +If unset, the default value for this parameter is 600. Setting it to 0
> +disables this feature.

Should this wording mention that the non-zero default is only on Linux 
(or rather, only on platforms where a non-zero value makes a difference)?

>   
>   Note that this functionality currently relies on the MADV_DONTNEED
>   argument for madvise() to actually free the memory. This is a
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ac3b48ee54..46dac23d2f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2895,7 +2895,8 @@
>   #
>   # @cache-clean-interval:  clean unused entries in the L2 and refcount
>   #                         caches. The interval is in seconds. The default value
> -#                         is 0 and it disables this feature (since 2.5)
> +#                         is 600, and 0 disables this feature. (since 2.5)

and again here

> +#
>   # @encrypt:               Image decryption options. Mandatory for
>   #                         encrypted images, except when doing a metadata-only
>   #                         probe of the image. (since 2.10)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 14aee78c6c..52d9d9f06d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -747,7 +747,7 @@ it which is not used for the L2 cache)
>   
>   @item cache-clean-interval
>   Clean unused entries in the L2 and refcount caches. The interval is in seconds.
> -The default value is 0 and it disables this feature.
> +The default value is 600. Setting it to 0 disables this feature.

and here?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
Posted by Leonid Bloch 7 years, 4 months ago

On September 27, 2018 5:53:34 PM CEST, Eric Blake <eblake@redhat.com> wrote:
>On 9/26/18 11:04 AM, Leonid Bloch wrote:
>> The default cache-clean-interval is set to 10 minutes, in order to
>lower
>> the overhead of the qcow2 caches (before the default was 0, i.e.
>> disabled).
>> 
>> * For non-Linux platforms the default is kept at 0, because
>>    cache-clean-interval is not supported there yet.
>> 
>> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>
>> @@ -76,13 +76,15 @@
>>   
>>   #ifdef CONFIG_LINUX
>>   #define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
>> +#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
>>   #else
>>   #define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
>> +/* Cache clean interval is currently available only on Linux, so
>must be 0 */
>> +#define DEFAULT_CACHE_CLEAN_INTERVAL 0
>>   #endif
>>   
>
>> +++ b/docs/qcow2-cache.txt
>> @@ -210,8 +210,8 @@ This example removes all unused cache entries
>every 15 minutes:
>>   
>>      -drive file=hd.qcow2,cache-clean-interval=900
>>   
>> -If unset, the default value for this parameter is 0 and it disables
>> -this feature.
>> +If unset, the default value for this parameter is 600. Setting it to
>0
>> +disables this feature.
>
>Should this wording mention that the non-zero default is only on Linux 
>(or rather, only on platforms where a non-zero value makes a
>difference)?

Yes, I think it should mention it, thanks. Should I send a separate patch?

Leonid.

>
>>   
>>   Note that this functionality currently relies on the MADV_DONTNEED
>>   argument for madvise() to actually free the memory. This is a
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index ac3b48ee54..46dac23d2f 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2895,7 +2895,8 @@
>>   #
>>   # @cache-clean-interval:  clean unused entries in the L2 and
>refcount
>>   #                         caches. The interval is in seconds. The
>default value
>> -#                         is 0 and it disables this feature (since
>2.5)
>> +#                         is 600, and 0 disables this feature.
>(since 2.5)
>
>and again here
>
>> +#
>>   # @encrypt:               Image decryption options. Mandatory for
>>   #                         encrypted images, except when doing a
>metadata-only
>>   #                         probe of the image. (since 2.10)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 14aee78c6c..52d9d9f06d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -747,7 +747,7 @@ it which is not used for the L2 cache)
>>   
>>   @item cache-clean-interval
>>   Clean unused entries in the L2 and refcount caches. The interval is
>in seconds.
>> -The default value is 0 and it disables this feature.
>> +The default value is 600. Setting it to 0 disables this feature.
>
>and here?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
Posted by Kevin Wolf 7 years, 4 months ago
Am 28.09.2018 um 05:59 hat Leonid Bloch geschrieben:
> 
> 
> On September 27, 2018 5:53:34 PM CEST, Eric Blake <eblake@redhat.com> wrote:
> >On 9/26/18 11:04 AM, Leonid Bloch wrote:
> >> The default cache-clean-interval is set to 10 minutes, in order to
> >lower
> >> the overhead of the qcow2 caches (before the default was 0, i.e.
> >> disabled).
> >> 
> >> * For non-Linux platforms the default is kept at 0, because
> >>    cache-clean-interval is not supported there yet.
> >> 
> >> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
> >> Reviewed-by: Alberto Garcia <berto@igalia.com>
> >> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> >> ---
> >
> >> @@ -76,13 +76,15 @@
> >>   
> >>   #ifdef CONFIG_LINUX
> >>   #define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
> >> +#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
> >>   #else
> >>   #define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
> >> +/* Cache clean interval is currently available only on Linux, so
> >must be 0 */
> >> +#define DEFAULT_CACHE_CLEAN_INTERVAL 0
> >>   #endif
> >>   
> >
> >> +++ b/docs/qcow2-cache.txt
> >> @@ -210,8 +210,8 @@ This example removes all unused cache entries
> >every 15 minutes:
> >>   
> >>      -drive file=hd.qcow2,cache-clean-interval=900
> >>   
> >> -If unset, the default value for this parameter is 0 and it disables
> >> -this feature.
> >> +If unset, the default value for this parameter is 600. Setting it to
> >0
> >> +disables this feature.
> >
> >Should this wording mention that the non-zero default is only on Linux 
> >(or rather, only on platforms where a non-zero value makes a
> >difference)?
> 
> Yes, I think it should mention it, thanks. Should I send a separate patch?

Yes, please use a follow-up patch if you want to improve the
documentation.

Kevin

Re: [Qemu-devel] [PATCH v12 8/9] qcow2: Set the default cache-clean-interval to 10 minutes
Posted by Alberto Garcia 7 years, 4 months ago
On Thu 27 Sep 2018 05:53:34 PM CEST, Eric Blake wrote:
> On 9/26/18 11:04 AM, Leonid Bloch wrote:
>> The default cache-clean-interval is set to 10 minutes, in order to lower
>> the overhead of the qcow2 caches (before the default was 0, i.e.
>> disabled).
>> 
>> * For non-Linux platforms the default is kept at 0, because
>>    cache-clean-interval is not supported there yet.
>> 
>> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>
>> @@ -76,13 +76,15 @@
>>   
>>   #ifdef CONFIG_LINUX
>>   #define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
>> +#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
>>   #else
>>   #define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
>> +/* Cache clean interval is currently available only on Linux, so must be 0 */
>> +#define DEFAULT_CACHE_CLEAN_INTERVAL 0
>>   #endif
>>   
>
>> +++ b/docs/qcow2-cache.txt
>> @@ -210,8 +210,8 @@ This example removes all unused cache entries every 15 minutes:
>>   
>>      -drive file=hd.qcow2,cache-clean-interval=900
>>   
>> -If unset, the default value for this parameter is 0 and it disables
>> -this feature.
>> +If unset, the default value for this parameter is 600. Setting it to 0
>> +disables this feature.
>
> Should this wording mention that the non-zero default is only on Linux
> (or rather, only on platforms where a non-zero value makes a
> difference)?

The next paragraph in the same document already says that this is a
Linux-only feature, so perhaps it's not so important.

Berto