Re: [PATCH] zram: Prevent page allocation failure during zcomp_strm_alloc

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

Hello,

On (11/19/15 15:54), kyeongdon.kim wrote:
> When we use lzo/lz4 multi compression streams for zram,
> we might face page allocation failure. In order to make parallel
> compression private data, we should call kzalloc() with order 2/3
> in runtime. But if there is no order 2/3 size memory to allocate
> in that time, page allocation fails.
> 
> This patch makes new zstrm stream list in zcomp_create() or
> max_comp_streams set, which means calling (kzalloc()) in runtime
> is not needed. This prevents page alloc failure.
> 

The decision here is to not create unneded streams (streams are not
free) -- we create one we need it. And if allocation fails we just
wait for already existing stream to become available (and we are
guaranteed to have at least one stream)

zcomp_strm_multi_find
{
...
        while (1) {
                spin_lock(&zs->strm_lock);
                if (!list_empty(&zs->idle_strm)) {
                        zstrm = list_entry(zs->idle_strm.next,
                                        struct zcomp_strm, list);
                        list_del(&zstrm->list);
                        spin_unlock(&zs->strm_lock);
                        return zstrm;
                }
...
                /* allocate new zstrm stream */
                zs->avail_strm++;
                spin_unlock(&zs->strm_lock);

                zstrm = zcomp_strm_alloc(comp);
                if (!zstrm) {
                        spin_lock(&zs->strm_lock);
                        zs->avail_strm--;
                        spin_unlock(&zs->strm_lock);
                        wait_event(zs->strm_wait, !list_empty(&zs->idle_strm));
                        continue;
                }
                break;
        }
        return zstrm;
}


So, are you trying to fix the warning from zcomp_strm_multi_find()? What
prevents this warning from happening in zcomp_strm_multi_set_max_streams()
or zcomp_strm_multi_create()? e.g. I requested 64 streams and ended up not
having enought memory? The warning will still be there and the streams will
not be allocated. What's the gain?


(besides, your implementation is racy. ->list and ->avail_strm should
not be modified outside of ->strm_lock).

	-ss

> For reference a call trace :
> 
> Binder_1: page allocation failure: order:3, mode:0x10c0d0
> CPU: 0 PID: 424 Comm: Binder_1 Tainted: GW 3.10.49-perf-g991d02b-dirty #20
> Call trace:
> [<ffffffc0002069c8>] dump_backtrace+0x0/0x270
> [<ffffffc000206c48>] show_stack+0x10/0x1c
> [<ffffffc000cb51c8>] dump_stack+0x1c/0x28
> [<ffffffc0002bbfc8>] warn_alloc_failed+0xfc/0x11c
> [<ffffffc0002bf518>] __alloc_pages_nodemask+0x724/0x7f0
> [<ffffffc0002bf5f8>] __get_free_pages+0x14/0x5c
> [<ffffffc0002ed6a4>] kmalloc_order_trace+0x38/0xd8
> [<ffffffc0005d9738>] zcomp_lz4_create+0x2c/0x38
> [<ffffffc0005d78f0>] zcomp_strm_alloc+0x34/0x78
> [<ffffffc0005d7a58>] zcomp_strm_multi_find+0x124/0x1ec
> [<ffffffc0005d7c14>] zcomp_strm_find+0xc/0x18
> [<ffffffc0005d8fa0>] zram_bvec_rw+0x2fc/0x780
> [<ffffffc0005d9680>] zram_make_request+0x25c/0x2d4
> [<ffffffc00040f8ac>] generic_make_request+0x80/0xbc
> [<ffffffc00040f98c>] submit_bio+0xa4/0x15c
> [<ffffffc0002e8bb0>] __swap_writepage+0x218/0x230
> [<ffffffc0002e8c04>] swap_writepage+0x3c/0x4c
> [<ffffffc0002c7384>] shrink_page_list+0x51c/0x8d0
> [<ffffffc0002c7e88>] shrink_inactive_list+0x3f8/0x60c
> [<ffffffc0002c86c8>] shrink_lruvec+0x33c/0x4cc
> [<ffffffc0002c8894>] shrink_zone+0x3c/0x100
> [<ffffffc0002c8c10>] try_to_free_pages+0x2b8/0x54c
> [<ffffffc0002bf308>] __alloc_pages_nodemask+0x514/0x7f0
> [<ffffffc0002bf5f8>] __get_free_pages+0x14/0x5c
> [<ffffffc0003446cc>] proc_info_read+0x50/0xe4
> [<ffffffc0002f5204>] vfs_read+0xa0/0x12c
> [<ffffffc0002f59c8>] SyS_read+0x44/0x74
> DMA: 3397*4kB (MC) 26*8kB (RC) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB
> 0*512kB 0*1024kB 0*2048kB 0*4096kB = 13796kB
> 
> Signed-off-by: kyeongdon.kim <kyeongdon.kim@lge.com>
> ---
>  drivers/block/zram/zcomp.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
> index f1ff39a..45f39b9 100644
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -158,6 +158,21 @@ static bool zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
>  	struct zcomp_strm_multi *zs = comp->stream;
>  	struct zcomp_strm *zstrm;
>  
> +	if (zs->avail_strm < num_strm) {
> +		while (1) {
> +			/* allocate new zstrm stream */
> +			zs->avail_strm++;
> +			zstrm = zcomp_strm_alloc(comp);
> +			if (!zstrm) {
> +				zs->avail_strm--;
> +				kfree(zs);
> +				return false;
> +			}
> +			list_add(&zstrm->list, &zs->idle_strm);
> +			if (zs->avail_strm == num_strm)
> +				break;
> +		}
> +	}
>  	spin_lock(&zs->strm_lock);
>  	zs->max_strm = num_strm;
>  	/*
> @@ -209,12 +224,20 @@ static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
>  	zs->max_strm = max_strm;
>  	zs->avail_strm = 1;
>  
> -	zstrm = zcomp_strm_alloc(comp);
> -	if (!zstrm) {
> -		kfree(zs);
> -		return -ENOMEM;
> +	while (1) {
> +		/* allocate new zstrm stream */
> +		zs->avail_strm++;
> +		zstrm = zcomp_strm_alloc(comp);
> +		if (!zstrm) {
> +			zs->avail_strm--;
> +			kfree(zs);
> +			return -ENOMEM;
> +		}
> +		list_add(&zstrm->list, &zs->idle_strm);
> +		if (zs->avail_strm == max_strm)
> +			break;
>  	}
> -	list_add(&zstrm->list, &zs->idle_strm);
> +
>  	return 0;
>  }
>  
> -- 
> 1.8.2
>