Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

-Wpointer-bool-conversion in drivers/staging/wilc1000/linux_wlan.c #121

Closed
tpimh opened this issue Sep 19, 2018 · 3 comments
Closed

-Wpointer-bool-conversion in drivers/staging/wilc1000/linux_wlan.c #121

tpimh opened this issue Sep 19, 2018 · 3 comments
Assignees
@nathanchance
Labels
-Wpointer-bool-conversion [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20

Comments

@tpimh
Copy link

tpimh commented Sep 19, 2018 

drivers/staging/wilc1000/linux_wlan.c:272:20: warning: address of 'vif->ndev->dev' will always evaluate to 'true' [-Wpointer-bool-conversion]
        if (!(&vif->ndev->dev))
            ~  ~~~~~~~~~~~^~~
1 warning generated.
@tpimh tpimh added [BUG] linux A bug that should be fixed in the mainline kernel. low priority This bug is not critical and not a priority labels Sep 19, 2018
@nickdesaulniers nickdesaulniers added the good first issue Good for newcomers label Sep 19, 2018
@nathanchance
Copy link
Member

Patch sent: https://lore.kernel.org/lkml/20180920212648.25181-1-natechancellor@gmail.com/

@nathanchance nathanchance self-assigned this Sep 20, 2018
@nathanchance nathanchance added [PATCH] Exists There is a patch that fixes this issue [PATCH] Submitted A patch has been submitted for review and removed [PATCH] Exists There is a patch that fixes this issue labels Sep 20, 2018
@tpimh tpimh removed the good first issue Good for newcomers label Sep 25, 2018
@nathanchance
Copy link
Member

Patch accepted: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=8bdfc47f4ef186d73b2849fcdc065c78e20bd71a

@nathanchance nathanchance added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Sep 25, 2018
fengguang pushed a commit to 0day-ci/linux that referenced this issue Sep 26, 2018
@nathanchance @gregkh
staging: wilc1000: Remove unnecessary pointer check
8bdfc47 
Clang warns that the address of a pointer will always evaluated as true
in a boolean context:

drivers/staging/wilc1000/linux_wlan.c:267:20: warning: address of
'vif->ndev->dev' will always evaluate to 'true'
[-Wpointer-bool-conversion]
        if (!(&vif->ndev->dev))
            ~  ~~~~~~~~~~~^~~
1 warning generated.

Since this statement always evaluates to false due to the logical not,
remove it.

Link: ClangBuiltLinux#121
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Ajay Singh <ajay.kathat@microchip.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
@nathanchance
Copy link
Member

Merged into mainline: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8bdfc47f4ef186d73b2849fcdc065c78e20bd71a

@nathanchance nathanchance added [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Oct 29, 2018
@tpimh tpimh removed the low priority This bug is not critical and not a priority label Oct 29, 2018
nathanchance pushed a commit that referenced this issue Jul 15, 2019
@jmberg-intel @richardweinberger
um: Silence lockdep complaint about mmap_sem
80bf6ce 
When we get into activate_mm(), lockdep complains that we're doing
something strange:

    WARNING: possible circular locking dependency detected
    5.1.0-10252-gb00152307319-dirty #121 Not tainted
    ------------------------------------------------------
    inside.sh/366 is trying to acquire lock:
    (____ptrval____) (&(&p->alloc_lock)->rlock){+.+.}, at: flush_old_exec+0x703/0x8d7

    but task is already holding lock:
    (____ptrval____) (&mm->mmap_sem){++++}, at: flush_old_exec+0x6c5/0x8d7

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&mm->mmap_sem){++++}:
           [...]
           __lock_acquire+0x12ab/0x139f
           lock_acquire+0x155/0x18e
           down_write+0x3f/0x98
           flush_old_exec+0x748/0x8d7
           load_elf_binary+0x2ca/0xddb
           [...]

    -> #0 (&(&p->alloc_lock)->rlock){+.+.}:
           [...]
           __lock_acquire+0x12ab/0x139f
           lock_acquire+0x155/0x18e
           _raw_spin_lock+0x30/0x83
           flush_old_exec+0x703/0x8d7
           load_elf_binary+0x2ca/0xddb
           [...]

    other info that might help us debug this:

     Possible unsafe locking scenario:

           CPU0                    CPU1
           ----                    ----
      lock(&mm->mmap_sem);
                                   lock(&(&p->alloc_lock)->rlock);
                                   lock(&mm->mmap_sem);
      lock(&(&p->alloc_lock)->rlock);

     *** DEADLOCK ***

    2 locks held by inside.sh/366:
     #0: (____ptrval____) (&sig->cred_guard_mutex){+.+.}, at: __do_execve_file+0x12d/0x869
     #1: (____ptrval____) (&mm->mmap_sem){++++}, at: flush_old_exec+0x6c5/0x8d7

    stack backtrace:
    CPU: 0 PID: 366 Comm: inside.sh Not tainted 5.1.0-10252-gb00152307319-dirty #121
    Stack:
     [...]
    Call Trace:
     [<600420de>] show_stack+0x13b/0x155
     [<6048906b>] dump_stack+0x2a/0x2c
     [<6009ae64>] print_circular_bug+0x332/0x343
     [<6009c5c6>] check_prev_add+0x669/0xdad
     [<600a06b4>] __lock_acquire+0x12ab/0x139f
     [<6009f3d0>] lock_acquire+0x155/0x18e
     [<604a07e0>] _raw_spin_lock+0x30/0x83
     [<60151e6a>] flush_old_exec+0x703/0x8d7
     [<601a8eb8>] load_elf_binary+0x2ca/0xddb
     [...]

I think it's because in exec_mmap() we have

	down_read(&old_mm->mmap_sem);
...
        task_lock(tsk);
...
	activate_mm(active_mm, mm);
	(which does down_write(&mm->mmap_sem))

I'm not really sure why lockdep throws in the whole knowledge
about the task lock, but it seems that old_mm and mm shouldn't
ever be the same (and it doesn't deadlock) so tell lockdep that
they're different.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nathanchance nathanchance

Labels
-Wpointer-bool-conversion [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 4.20 This bug was fixed in Linux 4.20
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tpimh @nickdesaulniers @nathanchance