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
