Re: [PATCH v2 08/10] softmmu/physmem: Extend ram_block_discard_(require|disable) by two discard types

[Pankaj Gupta]

> We want to separate the two cases whereby we discard ram
> - uncoordinated: e.g., virito-balloon
> - coordinated: e.g., virtio-mem coordinated via the RamDiscardMgr
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Auger Eric <eric.auger@redhat.com>
> Cc: Wei Yang <richard.weiyang@linux.alibaba.com>
> Cc: teawater <teawaterz@linux.alibaba.com>
> Cc: Marek Kedzierski <mkedzier@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  include/exec/memory.h | 18 +++++++++++++--
>  softmmu/physmem.c     | 54 ++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 64 insertions(+), 8 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 30d4fcd2c0..3f0942aa5b 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2804,6 +2804,12 @@ static inline MemOp devend_memop(enum device_endian 
> end)
>   */
>  int ram_block_discard_disable(bool state);
>
> +/*
> + * See ram_block_discard_disable(): only disable unccordinated discards,
s/unccordinated/uncoordinated
> + * keeping coordinated discards (via the RamDiscardMgr) enabled.
> + */
> +int ram_block_uncoordinated_discard_disable(bool state);
> +
>  /*
>   * Inhibit technologies that disable discarding of pages in RAM blocks.
>   *
> @@ -2813,12 +2819,20 @@ int ram_block_discard_disable(bool state);
>  int ram_block_discard_require(bool state);
>
>  /*
> - * Test if discarding of memory in ram blocks is disabled.
> + * See ram_block_discard_require(): only inhibit technologies that disable
> + * uncoordinated discarding of pages in RAM blocks, allowing co-existance 
> with
> + * technologies that only inhibit uncoordinated discards (via the
> + * RamDiscardMgr).
> + */
> +int ram_block_coordinated_discard_require(bool state);
> +
> +/*
> + * Test if any discarding of memory in ram blocks is disabled.
>   */
>  bool ram_block_discard_is_disabled(void);
>
>  /*
> - * Test if discarding of memory in ram blocks is required to work reliably.
> + * Test if any discarding of memory in ram blocks is required to work 
> reliably.
>   */
>  bool ram_block_discard_is_required(void);
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 448e4e8c86..7a4f3db1b4 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -3650,8 +3650,14 @@ void mtree_print_dispatch(AddressSpaceDispatch *d, 
> MemoryRegion *root)
>      }
>  }
>
> +/* Require any discards to work. */
>  static unsigned int ram_block_discard_requirers;
> +/* Require only coordinated discards to work. */
> +static unsigned int ram_block_coordinated_discard_requirers;
> +/* Disable any discards. */
>  static unsigned int ram_block_discard_disablers;
> +/* Disable only uncoordinated disards. */
s/disards/discards
> +static unsigned int ram_block_uncoordinated_discard_disablers;
>  static QemuMutex ram_block_discard_disable_mutex;
>
>  static void ram_block_discard_disable_mutex_lock(void)
> @@ -3677,10 +3683,27 @@ int ram_block_discard_disable(bool state)
>      ram_block_discard_disable_mutex_lock();
>      if (!state) {
>          ram_block_discard_disablers--;
> -    } else if (!ram_block_discard_requirers) {
> -        ram_block_discard_disablers++;
> +    } else if (ram_block_discard_requirers ||
> +               ram_block_coordinated_discard_requirers) {
> +        ret = -EBUSY;
>      } else {
> +        ram_block_discard_disablers++;
> +    }
> +    ram_block_discard_disable_mutex_unlock();
> +    return ret;
> +}
> +
> +int ram_block_uncoordinated_discard_disable(bool state)
> +{
> +    int ret = 0;
> +
> +    ram_block_discard_disable_mutex_lock();
> +    if (!state) {
> +        ram_block_uncoordinated_discard_disablers--;
> +    } else if (ram_block_discard_requirers) {
>          ret = -EBUSY;
> +    } else {
> +        ram_block_uncoordinated_discard_disablers++;
>      }
>      ram_block_discard_disable_mutex_unlock();
>      return ret;
> @@ -3693,10 +3716,27 @@ int ram_block_discard_require(bool state)
>      ram_block_discard_disable_mutex_lock();
>      if (!state) {
>          ram_block_discard_requirers--;
> -    } else if (!ram_block_discard_disablers) {
> -        ram_block_discard_requirers++;
> +    } else if (ram_block_discard_disablers ||
> +               ram_block_uncoordinated_discard_disablers) {
> +        ret = -EBUSY;
>      } else {
> +        ram_block_discard_requirers++;
> +    }
> +    ram_block_discard_disable_mutex_unlock();
> +    return ret;
> +}
> +
> +int ram_block_coordinated_discard_require(bool state)
> +{
> +    int ret = 0;
> +
> +    ram_block_discard_disable_mutex_lock();
> +    if (!state) {
> +        ram_block_coordinated_discard_requirers--;
> +    } else if (ram_block_discard_disablers) {
>          ret = -EBUSY;
> +    } else {
> +        ram_block_coordinated_discard_requirers++;
>      }
>      ram_block_discard_disable_mutex_unlock();
>      return ret;
> @@ -3704,10 +3744,12 @@ int ram_block_discard_require(bool state)
>
>  bool ram_block_discard_is_disabled(void)
>  {
> -    return qatomic_read(&ram_block_discard_disablers);
> +    return qatomic_read(&ram_block_discard_disablers) ||
> +           qatomic_read(&ram_block_uncoordinated_discard_disablers);
>  }
>
>  bool ram_block_discard_is_required(void)
>  {
> -    return qatomic_read(&ram_block_discard_requirers);
> +    return qatomic_read(&ram_block_discard_requirers) ||
> +           qatomic_read(&ram_block_coordinated_discard_requirers);
>  }

How to differentiate if we have both un-coordinated & coordinated
cases together?