On Fri, Apr 26, 2024 at 11:21:36AM +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]>
let's just delete the assertion - I don't like moving around reading the
lock state for this.
> ---
> v2:
> - Fix the error about reading lock->state introduced in v1.
>
> v1:
> https://lore.kernel.org/all/[email protected]/T/#u
> ---
> ---
> fs/bcachefs/six.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c
> index 3a494c5d1247..8bfd22a6f771 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:
> @@ -182,7 +182,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
>
