Re: [PATCH v2 05/11] zram: check mem_limit before allocating zsmalloc object

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/11/10 18:37), sashiko-bot@kernel.org wrote:
[..]
> [Severity: Low]
> Can zram_can_store_page() return false when mem_limit is cleared to 0 at
> runtime?
> 
> The write path does not hold zram->dev_lock, while mem_limit_store()
> updates zram->limit_pages under it.  Because zs_get_total_pages() is an
> external call, the compiler reloads zram->limit_pages afterwards:
> 
> CPU0                              CPU1
> zram_write_page()
>   zram_can_store_page()
>     if (!zram->limit_pages)
>       /* sees non-zero limit */
>     zs_get_total_pages(...)
>                                   mem_limit_store()
>                                     zram->limit_pages = 0;
>     return pages <= zram->limit_pages;
>       /* pages <= 0 is false */
> 
> When at least one page is allocated in zram->mem_pool, disabling the limit
> causes an in-flight write that was within the previous limit to fail with
> -ENOMEM.  Reading zram->limit_pages once into a local variable with
> READ_ONCE() avoids the second load.

I guess I can READ_ONCE(), but this scenario is extremely unlikely.
(I'm not even sure if anyone is using ->mem_limit.)