Re: [PATCH v2 1/1] xen/blkback: Aggressively shrink page pools if a memory pressure is detected

[sjpark]

On 05.12.19 16:09, SeongJae Park wrote:
> From: SeongJae Park <sjpark@xxxxxxxxx>
>
> Each `blkif` has a free pages pool for the grant mapping.  The size of
> the pool starts from zero and be increased on demand while processing
> the I/O requests.  If current I/O requests handling is finished or 100
> milliseconds has passed since last I/O requests handling, it checks and
> shrinks the pool to not exceed the size limit, `max_buffer_pages`.
>
> Therefore, `blkfront` running guests can cause a memory pressure in the
> `blkback` running guest by attaching a large number of block devices and
> inducing I/O.  System administrators can avoid such problematic
> situations by limiting the maximum number of devices each guest can
> attach.  However, finding the optimal limit is not so easy.  Improper
> set of the limit can result in the memory pressure or a resource
> underutilization.  This commit avoids such problematic situations by
> shrinking the pools aggressively (further the limit) for a while (users
> can set this duration via a module parameter) if a memory pressure is
> detected.
>
> Discussions
> ===========
>
> The `blkback`'s original shrinking mechanism returns only pages in the
> pool which are not currently be used by `blkback`.  In other words, the
> pages that will be shrunk are not mapped with foreign pages.  Because
> this commit is changing only the shrink limit but uses the shrinking
> mechanism as is, this commit does not introduce improper mappings
> related security issues.
>
> Once a memory pressure is detected, this commit keeps the aggressive
> shrinking limit for a user-specified time duration.  The duration should
> be neither too long nor too short.  If it is too long, free pages pool
> shrinking overhead can reduce the I/O performance.  If it is too short,
> `blkback` will not free enough pages to reduce the memory pressure.
> This commit sets the value as `10 milliseconds` by default because it is
> a short time in terms of I/O while it is a long time in terms of memory
> operations.  Also, as the original shrinking mechanism works for at
> least every 100 milliseconds, this could be a somewhat reasonable
> choice.  I also tested other durations (refer to the below section for
> more details) and confirmed that 10 milliseconds is the one that works
> best.  That said, the proper duration depends on actual configurations
> and workloads.  That's why this commit is allowing users to set it as
> their optimal value via the module parameter.
>
> Memory Pressure Test
> ====================
>
> To show how this commit fixes the above mentioned memory pressure
> situation well, I configured a test environment on a xen-running system.
> On the `blkfront` running guest instances, I attach a large number of
> network-backed volume devices and induce I/O to those.  Meanwhile, I
> measure the number of pages that swapped in and out on the `blkback`
> running guest.  The test ran twice, once for the `blkback` before this
> commit and once for that after this commit.  As shown below, this commit
> has dramatically reduced the memory pressure:
>
>                 pswpin  pswpout
>     before      76,672  185,799
>     after          212    3,325
>
> Optimal Aggressive Shrinking Duration
> -------------------------------------
>
> To find a best aggressive shrinking duration, I repeated the test with
> three different durations (1ms, 10ms, and 100ms).  The results are as
> below:
>
>     duration    pswpin  pswpout
>     1           852     6,424
>     10          212     3,325
>     100         203     3,340
>
> As expected, the numbers have further decreased by increasing the
> duration, but the reduction stopped from the `10ms`.  Based on this
> results, I chose the default duration as 10ms.
>
> Performance Overhead Test
> =========================
>
> This commit could incur I/O performance degradation under severe memory
> pressure because the aggressive shrinking will require more page
> allocations per I/O.  To show the overhead, I artificially made an
> aggressive pages pool shrinking situation and measured the I/O
> performance of a `blkfront` running guest.
>
> For the artificial shrinking, I set the `blkback.max_buffer_pages` using
> the `/sys/module/xen_blkback/parameters/max_buffer_pages` file.  We set
> the value to `1024` and `0`.  The `1024` is the default value.  Setting
> the value as `0` is same to a situation doing the aggressive shrinking
> always (worst-case).
>
> For the I/O performance measurement, I use a simple `dd` command.
>
> Default Performance
> -------------------
>
>     [dom0]# echo 1024 >
> /sys/module/xen_blkback/parameters/max_buffer_pages
>     [instance]$ for i in {1..5}; do dd if=/dev/zero of=file bs=4k
> count=$((256*512)); sync; done
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 11.7257 s, 45.8 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8827 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8781 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8737 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8702 s, 38.7 MB/s
>
> Worst-case Performance
> ----------------------
>
>     [dom0]# echo 0 >
> /sys/module/xen_blkback/parameters/max_buffer_pages
>     [instance]$ for i in {1..5}; do dd if=/dev/zero of=file bs=4k
> count=$((256*512)); sync; done
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 11.7257 s, 45.8 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.878 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8746 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8786 s, 38.7 MB/s
>     131072+0 records in
>     131072+0 records out
>     536870912 bytes (537 MB) copied, 13.8749 s, 38.7 MB/s
>
> In short, even worst case aggressive shrinking makes no visible
> performance degradation.  I think this is due to the slow speed of the
> I/O.  In other words, the additional page allocation overhead is hidden
> under the much slower I/O latency.
>
> Nevertheless, pleaset note that this is just a very simple and minimal
> test.
>
> Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>
> ---
>  drivers/block/xen-blkback/blkback.c | 35 +++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 3666afa639d1..72d068328ef1 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -135,6 +135,31 @@ module_param(log_stats, int, 0644);
>  /* Number of free pages to remove on each call to gnttab_free_pages */
>  #define NUM_BATCH_FREE_PAGES 10
>  
> +/*
> + * Once a memory pressure is detected, keep aggressive shrinking of the free
> + * page pools for this time (milliseconds)
> + */
> +static int xen_blkif_aggressive_shrinking_duration = 10;
> +module_param_named(aggressive_shrinking_duration,
> +		xen_blkif_aggressive_shrinking_duration, int, 0644);
> +MODULE_PARM_DESC(aggressive_shrinking_duration,
> +"Duration in ms to do aggressive shrinking when a memory pressure is detected");
> +
> +static unsigned long xen_blk_mem_pressure_end;
> +
> +static unsigned long blkif_shrink_count(struct shrinker *shrinker,
> +				struct shrink_control *sc)
> +{
> +	xen_blk_mem_pressure_end = jiffies +
> +		msecs_to_jiffies(xen_blkif_aggressive_shrinking_duration);
> +	return 0;
> +}
> +
> +static struct shrinker blkif_shrinker = {
> +	.count_objects = blkif_shrink_count,
> +	.seeks = DEFAULT_SEEKS,
> +};
> +
>  static inline bool persistent_gnt_timeout(struct persistent_gnt *persistent_gnt)
>  {
>  	return xen_blkif_pgrant_timeout &&
> @@ -656,8 +681,11 @@ int xen_blkif_schedule(void *arg)
>  			ring->next_lru = jiffies + msecs_to_jiffies(LRU_INTERVAL);
>  		}
>  
> -		/* Shrink if we have more than xen_blkif_max_buffer_pages */
> -		shrink_free_pagepool(ring, xen_blkif_max_buffer_pages);
> +		/* Shrink the free pages pool if it is too large. */
> +		if (time_before(jiffies, xen_blk_mem_pressure_end))
> +			shrink_free_pagepool(ring, 0);
> +		else
> +			shrink_free_pagepool(ring, xen_blkif_max_buffer_pages);
>  
>  		if (log_stats && time_after(jiffies, ring->st_print))
>  			print_stats(ring);
> @@ -1500,6 +1528,9 @@ static int __init xen_blkif_init(void)
>  	if (rc)
>  		goto failed_init;
>  
> +	if (register_shrinker(&blkif_shrinker))
> +		pr_warn("shrinker registration failed\n");
> +
>   failed_init:
>  	return rc;
>  }

CC-ing xen-devel@xxxxxxxxxxxxxxxxxxxx


Thanks,
SeongJae Park