Re: [PATCH 4/7] zram: allocate zsmalloc object under slot lock

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/11/06 04:48), sashiko-bot@kernel.org wrote:
[..]
> > +	slot_lock(zram, index);
> > +	slot_free(zram, index);
> >  	handle = zs_malloc(zram->mem_pool, comp_len,
> >  			   GFP_NOIO | __GFP_NOWARN |
> >  			   __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
> >  	if (IS_ERR(handle)) {
> >  		zcomp_cstrm_put(zstrm);
> > +		slot_unlock(zram, index);
> >  		return PTR_ERR(handle);
> >  	}
> >  
> >  	if (!zram_can_store_page(zram)) {
> >  		zcomp_cstrm_put(zstrm);
> >  		zs_free(zram->mem_pool, handle);
> > +		slot_unlock(zram, index);
> >  		return -ENOMEM;
> >  	}
> 
> [Severity: Critical]
> Does calling slot_free() before zs_malloc() and zram_can_store_page() in
> write_incompressible_page() and zram_write_page() cause existing data in the
> slot to be lost if the allocation or memory limit check fails?

Realistically mem_limit is the only scenario to consider and I will
move mem_limit check before slot_free/zs_store.  The mem_limit was
never a hard limit anyway, overshooting it by one zspage is not a
huge problem.

The zs_store() failure is very unlikely in practice, because it implies
a 0-order page allocation failure.