Clarification on last_comp_len logic in zram_write_page

Clarification on last_comp_len logic in zram_write_page

[Dheeraj Reddy Jonnalagadda]

Dear Maintainers,

I am writing to seek clarification regarding the use of last_comp_len
variable in zram_write_page function. Specifically, Coverity has flagged 
the issue (CID 1602439) in zram/zram_drv.c

Link to the coverity issue:
https://scan7.scan.coverity.com/#/project-view/52337/11354?selectedIssue=1602439

Currently, last_comp_len is initialized to 0 but never updated within the
function. This renders the conditional block shown below as dead code.

	if (last_comp_len && (last_comp_len != comp_len)) {
    		zs_free(zram->mem_pool, handle);
    		handle = -ENOMEM;
	}

From the context, it appears that this variable might be intended to track the
compression length from a previous iteration for comparison purposes.

I would like to confirm the intended behavior here:

	1. Should last_comp_len be updated to comp_len after every compression
	 iteration to enable this comparison?

	2. Is this logic necessary for scenarios where compression lengths
	differ between iterations, or can this block be removed altogether?

To address this, I propose a fix where last_comp_len is updated after the
conditional block as shown below, ensuring it reflects the last compression
length:
	if (last_comp_len && (last_comp_len != comp_len)) {
    		zs_free(zram->mem_pool, handle);
    		andle = -ENOMEM;
	}
	last_comp_len = comp_len;

Could you please clarify whether this approach aligns with the intended
functionality of the code?

Thank you for your time and assistance.

-Dheeraj

Re: Clarification on last_comp_len logic in zram_write_page

[Sergey Senozhatsky]

On (24/12/13 12:49), Dheeraj Reddy Jonnalagadda wrote:
> I am writing to seek clarification regarding the use of last_comp_len
> variable in zram_write_page function. Specifically, Coverity has flagged 
> the issue (CID 1602439) in zram/zram_drv.c
> 
> Currently, last_comp_len is initialized to 0 but never updated within the
> function. This renders the conditional block shown below as dead code.
> 
> 	if (last_comp_len && (last_comp_len != comp_len)) {
>     		zs_free(zram->mem_pool, handle);
>     		handle = -ENOMEM;
> 	}

That's a "known issue" [1], I deleted one extra line during rebase.
However, I expect last_comp_len patch do get withdrawn soon [2].

[1] https://lore.kernel.org/linux-kernel/20241211100638.GA2228457@google.com
[2] https://lore.kernel.org/mm-commits/3awo2svbnsv2mvozhaqspwztgxhifphj7ffpmykc35py6wp6ol@xlt2q5qgv6c3