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@xxxxxxxxxxxx

> d) what was wrong with the original code?  And still a).
> 
> > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> 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.