[PATCH] zram: do not skip the first bucket

[PATCH] zram: do not skip the first bucket

[Sergey Senozhatsky]

A small fixup.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 drivers/block/zram/zram_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index d3329a67e805..263795c4aef7 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -264,7 +264,7 @@ static struct zram_pp_slot *select_pp_slot(struct zram_pp_ctl *ctl)
 	s32 idx = NUM_PP_BUCKETS - 1;
 
 	/* The higher the bucket id the more optimal slot post-processing is */
-	while (idx > 0) {
+	while (idx >= 0) {
 		pps = list_first_entry_or_null(&ctl->pp_buckets[idx],
 					       struct zram_pp_slot,
 					       entry);
-- 
2.46.1.824.gd892dcdcdd-goog

Re: [PATCH] zram: do not skip the first bucket

[Andrew Morton]

On Tue,  1 Oct 2024 17:55:56 +0900 Sergey Senozhatsky <senozhatsky@chromium.org> wrote:

> A small fixup.
> 
> ...
>
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -264,7 +264,7 @@ static struct zram_pp_slot *select_pp_slot(struct zram_pp_ctl *ctl)
>  	s32 idx = NUM_PP_BUCKETS - 1;
>  
>  	/* The higher the bucket id the more optimal slot post-processing is */
> -	while (idx > 0) {
> +	while (idx >= 0) {
>  		pps = list_first_entry_or_null(&ctl->pp_buckets[idx],
>  					       struct zram_pp_slot,
>  					       entry);

I hate to be a kernel bureaucrat, but there's a lot missing from this
changelog!

a) What are the user-visible runtime effects?

b) What is the Fixes:

c) Is a cc:stable needed?  If so, a) is super-relevant.

oh, it's a fix against the mm-unstable patch "zram: rework recompress
target selection strategy".  That's new information!  Please disregard
the above.

d) what was wrong with the original code?  And still a).

> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>

e) what did Dan report ("Closes:")?


Sorry, but this is all stuff which you easily had available but which I
had to figure out.  And which I now present to other readers so they
needn't figure it out.  That would be inefficient!

Ho hum, anyway, thanks, applied as an effectively unchangelogged fix
against mm-unstable's "zram: rework recompress target selection
strategy".

Re: [PATCH] zram: do not skip the first bucket

[Sergey Senozhatsky]

On (24/10/01 14:57), Andrew Morton wrote:
> > A small fixup.
> > 
> > ...
> >
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -264,7 +264,7 @@ static struct zram_pp_slot *select_pp_slot(struct zram_pp_ctl *ctl)
> >  	s32 idx = NUM_PP_BUCKETS - 1;
> >  
> >  	/* The higher the bucket id the more optimal slot post-processing is */
> > -	while (idx > 0) {
> > +	while (idx >= 0) {
> >  		pps = list_first_entry_or_null(&ctl->pp_buckets[idx],
> >  					       struct zram_pp_slot,
> >  					       entry);
> 
> I hate to be a kernel bureaucrat, but there's a lot missing from this
> changelog!

Oh, sorry.  I thought that would be just a fixup patch that gets
squashed with the patch it was applied against.

> a) What are the user-visible runtime effects?

There aren't too many.  Buckets are size classes that hold compressed
objects' indexes (zram slots) that are candidates for post-processing
(re-compression of writeback).  The bucket 0 was skipped before, which
is the bucket for compressed objects smaller than 64 bytes.  We rarely
have anything there, such level of compression (PAGE_SIZE -> 64 bytes)
is not common in general.  The lower the bucket index the less
interested we are in post-processing of the items there.  E.g.
recompression of a 64 bytes object with more efficient algorithm,
even if successful, probably will save us just a couple of bytes.

> b) What is the Fixes:

It doesn't fix any upstream commit, the code in question is in
mm-unstable.

> c) Is a cc:stable needed?  If so, a) is super-relevant.

No.  And a) is not super-relevant.

> oh, it's a fix against the mm-unstable patch "zram: rework recompress
> target selection strategy".  That's new information!  Please disregard
> the above.

Oh, yes, correct.  This series:
https://lore.kernel.org/linux-kernel/20240917021020.883356-1-senozhatsky@chromium.org

> d) what was wrong with the original code?  And still a).
> 
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> e) what did Dan report ("Closes:")?

It doesn't close any known/reported issue.  The Reported-by tag there
is to give Dan credit for spotting that "typo".

> Sorry, but this is all stuff which you easily had available but which I
> had to figure out.  And which I now present to other readers so they
> needn't figure it out.  That would be inefficient!

My bad, sir.

> Ho hum, anyway, thanks, applied as an effectively unchangelogged fix
> against mm-unstable's "zram: rework recompress target selection
> strategy".

Thank you.