Skip to contentSkip to conversation

Clarification on last_comp_len logic in zram_write_page

DISCUSSION Dheeraj Reddy Jonnalagadda 2 participants 2 messages Dec 13, 2024 Block layer

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 11 lines · Dheeraj Reddy Jonnalagadda

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/[email protected]
[2] https://lore.kernel.org/mm-commits/3awo2svbnsv2mvozhaqspwztgxhifphj7ffpmykc35py6wp6ol@xlt2q5qgv6c3