Re: [PATCH] bcachefs: initialize local variables in bch2_evacuate_bucket

[Kent Overstreet]

On Sun, Nov 17, 2024 at 06:00:40PM -0700, Nathan Chancellor wrote:
> On Sun, Nov 17, 2024 at 11:48:23PM +0000, Piotr Zalewski wrote:
> > Compiling bcachefs sources with LLVM triggers uninitialized variables
> > warnings.
> > 
> > Signed-off-by: Piotr Zalewski <pZ010001011111@xxxxxxxxx>
> > ---
> >  fs/bcachefs/move.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c
> > index 8c032ef3a567..94cac498d372 100644
> > --- a/fs/bcachefs/move.c
> > +++ b/fs/bcachefs/move.c
> > @@ -674,8 +674,8 @@ int bch2_evacuate_bucket(struct moving_context *ctxt,
> >  	struct bkey_buf sk;
> >  	struct bkey_s_c k;
> >  	struct data_update_opts data_opts;
> > -	unsigned dirty_sectors, bucket_size;
> > -	u64 fragmentation;
> > +	unsigned dirty_sectors = 0, bucket_size = 0;
> > +	u64 fragmentation = 0;
> 
> Shouldn't these just be removed altogether...? They are only used in a
> trace event and now only zero.

Actually, I think we want to bring them back; being able to understand
the behaviour of these background tasks is important whenever we have to
debug something.

Hasn't come up as much with copygc recently, but it has with rebalance
and I was just improving the rebalance tracepoint.

(The copygc issue that does come up is that it behaves poorly with a
multi device tiering filesystem where it's rebalance's job to move data
off a full foreground device, not copygc, but that doesn't hit this
codepath).

But - fragmentation is not really a "evacuate_bucket()" concern, that's
a generic codepath, so I think I'll do something simpler here and then
later we can add a dedicated copygc tracepoint, and that can include
information about the fragmentation LRU that copygc looks at.

So...