On Fri, Apr 19, 2024 at 03:48:51PM +0800, Hongbo Li wrote:
> When compiling the bcachefs-tools, the following compilation warning
> is reported:
> libbcachefs/six.c: In function ‘__do_six_trylock’:
> libbcachefs/six.c:90:12: warning: ‘old’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> 90 | if (!(old & SIX_LOCK_HELD_intent)) {
> | ~~~~~^~~~~~~~~~~~~~~~~~~~~~~
>
> This is also a false altert. Only when @type=SIX_LOCK_write and @try=false
> are passed in __do_six_trylock, the second condition branch would enter
> which does not initialize the @old variable. But six_set_owner will not
> use @old if @type is not SIX_LOCK_intent. There should be nothing wrong
> in logical too.
>
> Although the report itself is a false alert, we can elimate the unitialize
> compilation warning by assigning @old in front.
>
> Fixes: 84a37cbf62e0 ("six locks: Wakeup now takes lock on behalf of waiter")
> Signed-off-by: Hongbo Li <[email protected]>
> ---
> fs/bcachefs/six.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c
> index 3a494c5d1247..9f782e4e3ca9 100644
> --- a/fs/bcachefs/six.c
> +++ b/fs/bcachefs/six.c
> @@ -118,11 +118,11 @@ static int __do_six_trylock(struct six_lock *lock, enum
> six_lock_type type,
> struct task_struct *task, bool try)
> {
> int ret;
> - u32 old;
> + u32 old = atomic_read(&lock->state);
>
> EBUG_ON(type == SIX_LOCK_write && lock->owner != task);
> EBUG_ON(type == SIX_LOCK_write &&
> - (try != !(atomic_read(&lock->state) & SIX_LOCK_HELD_write)));
> + (try != !(old & SIX_LOCK_HELD_write)));
>
> /*
> * Percpu reader mode:
> @@ -157,7 +157,6 @@ static int __do_six_trylock(struct six_lock *lock, enum
> six_lock_type type,
>
> smp_mb();
>
> - old = atomic_read(&lock->state);
Nope, this is wrong. That smp_mb() is there for a reason.
> ret = !(old & l[type].lock_fail);
>
> this_cpu_sub(*lock->readers, !ret);
> @@ -182,7 +181,6 @@ static int __do_six_trylock(struct six_lock *lock, enum
> six_lock_type type,
> ret = -1 - SIX_LOCK_read;
> }
> } else {
> - old = atomic_read(&lock->state);
> do {
> ret = !(old & l[type].lock_fail);
> if (!ret || (type == SIX_LOCK_write && !try)) {
> --
> 2.34.1
>
