All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Minchan Kim <minchan@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nitin Gupta <ngupta@vflare.org>,
	Suleiman Souhlal <suleiman@google.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	llvm@lists.linux.dev
Subject: Re: [PATCHv5 04/13] zram: Introduce recompress sysfs knob
Date: Thu, 10 Nov 2022 06:09:33 -0700	[thread overview]
Message-ID: <Y2z4DbuYgDJ/v8u+@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20221109115047.2921851-5-senozhatsky@chromium.org>

Hi Sergey,

On Wed, Nov 09, 2022 at 08:50:38PM +0900, Sergey Senozhatsky wrote:
> Allow zram to recompress (using secondary compression streams)
> pages.
> 
> Re-compression algorithms (we support up to 3 at this stage)
> are selected via recomp_algorithm:
> 
>   echo "algo=zstd priority=1" > /sys/block/zramX/recomp_algorithm
> 
> Please read documentation for more details.
> 
> We support several recompression modes:
> 
> 1) IDLE pages recompression is activated by `idle` mode
> 
>   echo "type=idle" > /sys/block/zram0/recompress
> 
> 2) Since there may be many idle pages user-space may pass a size
> threshold value (in bytes) and we will recompress pages only
> of equal or greater size:
> 
>   echo "threshold=888" > /sys/block/zram0/recompress
> 
> 3) HUGE pages recompression is activated by `huge` mode
> 
>   echo "type=huge" > /sys/block/zram0/recompress
> 
> 4) HUGE_IDLE pages recompression is activated by `huge_idle` mode
> 
>   echo "type=huge_idle" > /sys/block/zram0/recompress
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
...
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index a6a5fd2474d7..749e4266dd72 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
...
> +	for (index = 0; index < nr_pages; index++) {
> +		int err;
> +
> +		zram_slot_lock(zram, index);
> +
> +		if (!zram_allocated(zram, index))
> +			goto next;
> +
> +		if (mode & RECOMPRESS_IDLE &&
> +		    !zram_test_flag(zram, index, ZRAM_IDLE))
> +			goto next;
> +
> +		if (mode & RECOMPRESS_HUGE &&
> +		    !zram_test_flag(zram, index, ZRAM_HUGE))
> +			goto next;
> +
> +		if (zram_test_flag(zram, index, ZRAM_WB) ||
> +		    zram_test_flag(zram, index, ZRAM_UNDER_WB) ||
> +		    zram_test_flag(zram, index, ZRAM_SAME) ||
> +		    zram_test_flag(zram, index, ZRAM_INCOMPRESSIBLE))
> +			goto next;
> +
> +		err = zram_recompress(zram, index, page, threshold,
> +				      prio, ZRAM_MAX_COMPS);
> +next:
> +		zram_slot_unlock(zram, index);
> +		if (err) {
> +			ret = err;
> +			break;
> +		}
> +
> +		cond_resched();
> +	}

This commit is now in -next as commit 03e6c729aa64 ("zram: introduce
recompress sysfs knob"), where it introduces the following clang
warnings:

drivers/block/zram/zram_drv.c:1909:7: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1909:3: note: remove the 'if' if its condition is always false
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1909:7: error: variable 'err' is used uninitialized whenever '||' condition is true [-Werror,-Wsometimes-uninitialized]
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1909:7: note: remove the '||' if its condition is always false
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1909:7: error: variable 'err' is used uninitialized whenever '||' condition is true [-Werror,-Wsometimes-uninitialized]
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1909:7: note: remove the '||' if its condition is always false
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1909:7: error: variable 'err' is used uninitialized whenever '||' condition is true [-Werror,-Wsometimes-uninitialized]
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1909:7: note: remove the '||' if its condition is always false
                if (zram_test_flag(zram, index, ZRAM_WB) ||
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1905:7: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
                if (mode & RECOMPRESS_HUGE &&
                    ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1905:3: note: remove the 'if' if its condition is always false
                if (mode & RECOMPRESS_HUGE &&
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1901:7: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
                if (mode & RECOMPRESS_IDLE &&
                    ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1901:3: note: remove the 'if' if its condition is always false
                if (mode & RECOMPRESS_IDLE &&
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1898:7: error: variable 'err' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
                if (!zram_allocated(zram, index))
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1919:7: note: uninitialized use occurs here
                if (err) {
                    ^~~
drivers/block/zram/zram_drv.c:1898:3: note: remove the 'if' if its condition is always false
                if (!zram_allocated(zram, index))
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/zram/zram_drv.c:1894:10: note: initialize the variable 'err' to silence this warning
                int err;
                       ^
                        = 0
7 errors generated.

Is the fix just to initialize err to 0 as it suggests or should there be
a different fix?

Cheers,
Nathan

  reply	other threads:[~2022-11-10 13:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 11:50 [PATCHv5 00/13] zram: Support multiple compression streams Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 01/13] zram: Preparation for multi-zcomp support Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 02/13] zram: Add recompression algorithm sysfs knob Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 03/13] zram: Factor out WB and non-WB zram read functions Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 04/13] zram: Introduce recompress sysfs knob Sergey Senozhatsky
2022-11-10 13:09   ` Nathan Chancellor [this message]
2022-11-10 14:31     ` Sergey Senozhatsky
2022-11-10 14:38       ` Sergey Senozhatsky
2022-11-10 15:18         ` Nathan Chancellor
2022-11-10 14:34   ` [PATCH] zram: we should always zero out err variable in recompress loop Sergey Senozhatsky
2022-11-14  2:14   ` [PATCH] zram: explicitly limit prio_max for static analyzers Sergey Senozhatsky
2022-11-15  0:41     ` Andrew Morton
2022-11-15  0:47       ` Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 05/13] zram: Add recompress flag to read_block_state() Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 06/13] zram: Clarify writeback_store() comment Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 07/13] zram: Use IS_ERR_VALUE() to check for zs_malloc() errors Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 08/13] zram: add size class equals check into recompression Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 09/13] zram: remove redundant checks from zram_recompress() Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 10/13] zram: Add algo parameter support to zram_recompress() Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 11/13] documentation: Add zram recompression documentation Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 12/13] zram: add incompressible writeback Sergey Senozhatsky
2022-11-09 11:50 ` [PATCHv5 13/13] zram: Add incompressible flag to read_block_state() Sergey Senozhatsky
2022-11-09 21:46 ` [PATCHv5 00/13] zram: Support multiple compression streams Minchan Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2z4DbuYgDJ/v8u+@dev-arch.thelio-3990X \
    --to=nathan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=llvm@lists.linux.dev \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=senozhatsky@chromium.org \
    --cc=suleiman@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.