Re: [PATCH 6.5 211/285] btrfs: scrub: fix grouping of read IO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2023/10/27 07:42, Sam James wrote:

Qu Wenruo <wqu@xxxxxxxx> writes:

On 2023/10/27 00:30, Holger Hoffstätte wrote:
On 2023-10-26 15:31, Sam James wrote:
'btrfs: scrub: fix grouping of read IO' seems to intorduce a
-Wmaybe-uninitialized warning (which becomes fatal with the kernel's
passed -Werror=...) with 6.5.9:

```
/var/tmp/portage/sys-kernel/gentoo-kernel-6.5.9/work/linux-6.5/fs/btrfs/scrub.c: In function ‘scrub_simple_mirror.isra’:
/var/tmp/portage/sys-kernel/gentoo-kernel-6.5.9/work/linux-6.5/fs/btrfs/scrub.c:2075:29:
error: ‘found_logical’ may be used uninitialized
[-Werror=maybe-uninitialized[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmaybe-uninitialized]]
   2075 |                 cur_logical = found_logical + BTRFS_STRIPE_LEN;
/var/tmp/portage/sys-kernel/gentoo-kernel-6.5.9/work/linux-6.5/fs/btrfs/scrub.c:2040:21: note: ‘found_logical’ was declared here
   2040 |                 u64 found_logical;
        |                     ^~~~~~~~~~~~~
```
Good find! found_logical is passed by reference to
queue_scrub_stripe(..) (inlined)
where it is used without ever being set:
...
      /* Either >0 as no more extents or <0 for error. */
      if (ret)
          return ret;
      if (found_logical_ret)
          *found_logical_ret = stripe->logical;
      sctx->cur_stripe++;
...
Something is missing here, and somehow I don't think it's just the
top-level
initialisation of found_logical.

This looks like a false alert for me.

@found_logical is intentionally uninitialized to catch any
uninitialized usage by compiler.


I feel like this sort of thing is going to be inevitable with -Wmaybe-uninitialized.

Indeed, it can be tricky for compilers to handle.

But it's not that frequent, thus the btrfs community is mostly fine to handle those reports manually.


It would be set by queue_scrub_stripe() when there is any stripe found.

If there is no stripe found, queue_scrub_stripe() would return >0,
then we know there is no more extent and break the loop.
If there is any error, we error out too, thus no problem with that.

So to me this looks like a false alert.

Mind to share the compiler and its version?


Sure, GCC 14.0.0 20231022 (experimental). Sorry, I'd meant to include
that in the original post..

Wow, the first time I can not catch up on the toolchain version...

And considering it's still experimental, I'd say if we got another report on this particular call site, we may consider add a fix for it, by simply initializing @found_logical to zero.

Thanks,
Qu

Thanks,
Qu
-h




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux