Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On Sun, May 31, 2020 at 05:18:06AM -0400, Michael S. Tsirkin wrote: > On Tue, May 26, 2020 at 08:53:43PM +0200, Greg Kroah-Hartman wrote: > > From: Michael S. Tsirkin > > > > [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ] > > > > This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031. > > > > It has been queued properly in the akpm tree, this version is just > > creating conflicts. > > > > Signed-off-by: Michael S. Tsirkin > > Signed-off-by: Sasha Levin > > I don't understand. How does this make sense in stable? > stable does not merge akpm does it? It does not make sense, and is queued up to be reverted in the next release. thanks, greg k-h
Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On Tue, May 26, 2020 at 08:53:43PM +0200, Greg Kroah-Hartman wrote:
> From: Michael S. Tsirkin
>
> [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ]
>
> This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031.
>
> It has been queued properly in the akpm tree, this version is just
> creating conflicts.
>
> Signed-off-by: Michael S. Tsirkin
> Signed-off-by: Sasha Levin
I don't understand. How does this make sense in stable?
stable does not merge akpm does it?
> ---
> drivers/virtio/virtio_balloon.c | 107 +++-
> 1 file changed, 63 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 44375a22307b..341458fd95ca 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -14,7 +14,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -28,9 +27,7 @@
> */
> #define VIRTIO_BALLOON_PAGES_PER_PAGE (unsigned)(PAGE_SIZE >>
> VIRTIO_BALLOON_PFN_SHIFT)
> #define VIRTIO_BALLOON_ARRAY_PFNS_MAX 256
> -/* Maximum number of (4k) pages to deflate on OOM notifications. */
> -#define VIRTIO_BALLOON_OOM_NR_PAGES 256
> -#define VIRTIO_BALLOON_OOM_NOTIFY_PRIORITY 80
> +#define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
>
> #define VIRTIO_BALLOON_FREE_PAGE_ALLOC_FLAG (__GFP_NORETRY | __GFP_NOWARN | \
>__GFP_NOMEMALLOC)
> @@ -115,11 +112,8 @@ struct virtio_balloon {
> /* Memory statistics */
> struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
>
> - /* Shrinker to return free pages - VIRTIO_BALLOON_F_FREE_PAGE_HINT */
> + /* To register a shrinker to shrink memory upon memory pressure */
> struct shrinker shrinker;
> -
> - /* OOM notifier to deflate on OOM - VIRTIO_BALLOON_F_DEFLATE_ON_OOM */
> - struct notifier_block oom_nb;
> };
>
> static struct virtio_device_id id_table[] = {
> @@ -794,13 +788,50 @@ static unsigned long shrink_free_pages(struct
> virtio_balloon *vb,
> return blocks_freed * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
> }
>
> +static unsigned long leak_balloon_pages(struct virtio_balloon *vb,
> + unsigned long pages_to_free)
> +{
> + return leak_balloon(vb, pages_to_free * VIRTIO_BALLOON_PAGES_PER_PAGE) /
> + VIRTIO_BALLOON_PAGES_PER_PAGE;
> +}
> +
> +static unsigned long shrink_balloon_pages(struct virtio_balloon *vb,
> + unsigned long pages_to_free)
> +{
> + unsigned long pages_freed = 0;
> +
> + /*
> + * One invocation of leak_balloon can deflate at most
> + * VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
> + * multiple times to deflate pages till reaching pages_to_free.
> + */
> + while (vb->num_pages && pages_freed < pages_to_free)
> + pages_freed += leak_balloon_pages(vb,
> + pages_to_free - pages_freed);
> +
> + update_balloon_size(vb);
> +
> + return pages_freed;
> +}
> +
> static unsigned long virtio_balloon_shrinker_scan(struct shrinker *shrinker,
> struct shrink_control *sc)
> {
> + unsigned long pages_to_free, pages_freed = 0;
> struct virtio_balloon *vb = container_of(shrinker,
> struct virtio_balloon, shrinker);
>
> - return shrink_free_pages(vb, sc->nr_to_scan);
> + pages_to_free = sc->nr_to_scan;
> +
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> + pages_freed = shrink_free_pages(vb, pages_to_free);
> +
> + if (pages_freed >= pages_to_free)
> + return pages_freed;
> +
> + pages_freed += shrink_balloon_pages(vb, pages_to_free - pages_freed);
> +
> + return pages_freed;
> }
>
> static unsigned long virtio_balloon_shrinker_count(struct shrinker *shrinker,
> @@ -808,22 +839,26 @@ static unsigned long
> virtio_balloon_shrinker_count(struct shrinker *shrinker,
> {
> struct virtio_balloon *vb = container_of(shrinker,
> struct virtio_balloon, shrinker);
> + unsigned long count;
> +
> + count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
> + count += vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
>
> - return vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
> + return count;
> }
>
> -static int virtio_balloon_oom_notify(struct notifier_block *nb,
> - unsigned long dummy, void *parm)
> +static void virtio_balloon_unregister_shrinker(struct virtio_balloon *vb)
> {
> - struct virtio_balloon *vb = container_of(nb,
> - struct virtio_balloon, oom_nb);
> - unsigned long *freed = parm;
> + unregister_shrinker(>shrinker);
> +}
>
> - *freed += leak_balloon(vb,
Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On Thu, May 28, 2020 at 07:11:17AM -0400, Sasha Levin wrote: > On Thu, May 28, 2020 at 10:21:41AM +0200, David Hildenbrand wrote: > > On 28.05.20 07:51, Jiri Slaby wrote: > > > On 26. 05. 20, 20:53, Greg Kroah-Hartman wrote: > > > > From: Michael S. Tsirkin > > > > > > > > [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ] > > > > > > > > This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031. > > > > > > > > It has been queued properly in the akpm tree, this version is just > > > > creating conflicts. > > > > > > Should this be applied to stable trees at all? > > > > > > To me, it occurs to be a revert to avoid conflicts, not to fix something? > > > > Agreed. > > Right, I'll drop it - thank you. Already committed, I'll go revert this now, thanks. greg k-h
Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On Thu, May 28, 2020 at 10:21:41AM +0200, David Hildenbrand wrote: On 28.05.20 07:51, Jiri Slaby wrote: On 26. 05. 20, 20:53, Greg Kroah-Hartman wrote: From: Michael S. Tsirkin [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ] This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031. It has been queued properly in the akpm tree, this version is just creating conflicts. Should this be applied to stable trees at all? To me, it occurs to be a revert to avoid conflicts, not to fix something? Agreed. Right, I'll drop it - thank you. -- Thanks, Sasha
Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On 28.05.20 07:51, Jiri Slaby wrote: > On 26. 05. 20, 20:53, Greg Kroah-Hartman wrote: >> From: Michael S. Tsirkin >> >> [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ] >> >> This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031. >> >> It has been queued properly in the akpm tree, this version is just >> creating conflicts. > > Should this be applied to stable trees at all? > > To me, it occurs to be a revert to avoid conflicts, not to fix something? Agreed. -- Thanks, David / dhildenb
Re: [PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
On 26. 05. 20, 20:53, Greg Kroah-Hartman wrote: > From: Michael S. Tsirkin > > [ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ] > > This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031. > > It has been queued properly in the akpm tree, this version is just > creating conflicts. Should this be applied to stable trees at all? To me, it occurs to be a revert to avoid conflicts, not to fix something? > Signed-off-by: Michael S. Tsirkin > Signed-off-by: Sasha Levin thanks, -- js suse labs
[PATCH 5.6 086/126] virtio-balloon: Revert "virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM"
From: Michael S. Tsirkin
[ Upstream commit 835a6a649d0dd1b1f46759eb60fff2f63ed253a7 ]
This reverts commit 5a6b4cc5b7a1892a8d7f63d6cbac6e0ae2a9d031.
It has been queued properly in the akpm tree, this version is just
creating conflicts.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Sasha Levin
---
drivers/virtio/virtio_balloon.c | 107 +++-
1 file changed, 63 insertions(+), 44 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 44375a22307b..341458fd95ca 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -14,7 +14,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -28,9 +27,7 @@
*/
#define VIRTIO_BALLOON_PAGES_PER_PAGE (unsigned)(PAGE_SIZE >>
VIRTIO_BALLOON_PFN_SHIFT)
#define VIRTIO_BALLOON_ARRAY_PFNS_MAX 256
-/* Maximum number of (4k) pages to deflate on OOM notifications. */
-#define VIRTIO_BALLOON_OOM_NR_PAGES 256
-#define VIRTIO_BALLOON_OOM_NOTIFY_PRIORITY 80
+#define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
#define VIRTIO_BALLOON_FREE_PAGE_ALLOC_FLAG (__GFP_NORETRY | __GFP_NOWARN | \
__GFP_NOMEMALLOC)
@@ -115,11 +112,8 @@ struct virtio_balloon {
/* Memory statistics */
struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
- /* Shrinker to return free pages - VIRTIO_BALLOON_F_FREE_PAGE_HINT */
+ /* To register a shrinker to shrink memory upon memory pressure */
struct shrinker shrinker;
-
- /* OOM notifier to deflate on OOM - VIRTIO_BALLOON_F_DEFLATE_ON_OOM */
- struct notifier_block oom_nb;
};
static struct virtio_device_id id_table[] = {
@@ -794,13 +788,50 @@ static unsigned long shrink_free_pages(struct
virtio_balloon *vb,
return blocks_freed * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
}
+static unsigned long leak_balloon_pages(struct virtio_balloon *vb,
+ unsigned long pages_to_free)
+{
+ return leak_balloon(vb, pages_to_free * VIRTIO_BALLOON_PAGES_PER_PAGE) /
+ VIRTIO_BALLOON_PAGES_PER_PAGE;
+}
+
+static unsigned long shrink_balloon_pages(struct virtio_balloon *vb,
+ unsigned long pages_to_free)
+{
+ unsigned long pages_freed = 0;
+
+ /*
+* One invocation of leak_balloon can deflate at most
+* VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
+* multiple times to deflate pages till reaching pages_to_free.
+*/
+ while (vb->num_pages && pages_freed < pages_to_free)
+ pages_freed += leak_balloon_pages(vb,
+ pages_to_free - pages_freed);
+
+ update_balloon_size(vb);
+
+ return pages_freed;
+}
+
static unsigned long virtio_balloon_shrinker_scan(struct shrinker *shrinker,
struct shrink_control *sc)
{
+ unsigned long pages_to_free, pages_freed = 0;
struct virtio_balloon *vb = container_of(shrinker,
struct virtio_balloon, shrinker);
- return shrink_free_pages(vb, sc->nr_to_scan);
+ pages_to_free = sc->nr_to_scan;
+
+ if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
+ pages_freed = shrink_free_pages(vb, pages_to_free);
+
+ if (pages_freed >= pages_to_free)
+ return pages_freed;
+
+ pages_freed += shrink_balloon_pages(vb, pages_to_free - pages_freed);
+
+ return pages_freed;
}
static unsigned long virtio_balloon_shrinker_count(struct shrinker *shrinker,
@@ -808,22 +839,26 @@ static unsigned long virtio_balloon_shrinker_count(struct
shrinker *shrinker,
{
struct virtio_balloon *vb = container_of(shrinker,
struct virtio_balloon, shrinker);
+ unsigned long count;
+
+ count = vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
+ count += vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
- return vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
+ return count;
}
-static int virtio_balloon_oom_notify(struct notifier_block *nb,
-unsigned long dummy, void *parm)
+static void virtio_balloon_unregister_shrinker(struct virtio_balloon *vb)
{
- struct virtio_balloon *vb = container_of(nb,
-struct virtio_balloon, oom_nb);
- unsigned long *freed = parm;
+ unregister_shrinker(>shrinker);
+}
- *freed += leak_balloon(vb, VIRTIO_BALLOON_OOM_NR_PAGES) /
- VIRTIO_BALLOON_PAGES_PER_PAGE;
- update_balloon_size(vb);
+static int virtio_balloon_register_shrinker(struct virtio_balloon *vb)
+{
+ vb->shrinker.scan_objects = virtio_balloon_shrinker_scan;
+ vb->shrinker.count_objects = virtio_balloon_shrinker_count;
+
