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

From: Nathan Chancellor
Date: Sun Nov 17 2024 - 20:00:51 EST


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.

$ rg 'dirty_sectors|bucket_size|fragmentation' fs/bcachefs/move.c
677: unsigned dirty_sectors, bucket_size;
678: u64 fragmentation;
814: trace_evacuate_bucket(c, &bucket, dirty_sectors, bucket_size, fragmentation, ret);

Something like below seems to compile.

Cheers,
Nathan

diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c
index 88ab9d7e1a1b..412ef9897361 100644
--- a/fs/bcachefs/move.c
+++ b/fs/bcachefs/move.c
@@ -674,8 +674,6 @@ 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;
int ret = 0;

struct bch_dev *ca = bch2_dev_tryget(c, bucket.inode);
@@ -811,7 +809,7 @@ int bch2_evacuate_bucket(struct moving_context *ctxt,
bch2_btree_iter_advance(&bp_iter);
}

- trace_evacuate_bucket(c, &bucket, dirty_sectors, bucket_size, fragmentation, ret);
+ trace_evacuate_bucket(c, &bucket, ret);
err:
bch2_trans_iter_exit(trans, &bp_iter);
bch2_dev_put(ca);
diff --git a/fs/bcachefs/trace.h b/fs/bcachefs/trace.h
index 5597b9d6297f..f67914367acf 100644
--- a/fs/bcachefs/trace.h
+++ b/fs/bcachefs/trace.h
@@ -846,18 +846,13 @@ TRACE_EVENT(move_data,
);

TRACE_EVENT(evacuate_bucket,
- TP_PROTO(struct bch_fs *c, struct bpos *bucket,
- unsigned sectors, unsigned bucket_size,
- u64 fragmentation, int ret),
- TP_ARGS(c, bucket, sectors, bucket_size, fragmentation, ret),
+ TP_PROTO(struct bch_fs *c, struct bpos *bucket, int ret),
+ TP_ARGS(c, bucket, ret),

TP_STRUCT__entry(
__field(dev_t, dev )
__field(u64, member )
__field(u64, bucket )
- __field(u32, sectors )
- __field(u32, bucket_size )
- __field(u64, fragmentation )
__field(int, ret )
),

@@ -865,17 +860,12 @@ TRACE_EVENT(evacuate_bucket,
__entry->dev = c->dev;
__entry->member = bucket->inode;
__entry->bucket = bucket->offset;
- __entry->sectors = sectors;
- __entry->bucket_size = bucket_size;
- __entry->fragmentation = fragmentation;
__entry->ret = ret;
),

- TP_printk("%d,%d %llu:%llu sectors %u/%u fragmentation %llu ret %i",
+ TP_printk("%d,%d %llu:%llu ret %i",
MAJOR(__entry->dev), MINOR(__entry->dev),
- __entry->member, __entry->bucket,
- __entry->sectors, __entry->bucket_size,
- __entry->fragmentation, __entry->ret)
+ __entry->member, __entry->bucket, __entry->ret)
);

TRACE_EVENT(copygc,