Re: BUG: using smp_processor_id() in preemptible [00000000] during put_task_stack

From: Michal Hocko
Date: Mon Dec 12 2016 - 04:32:42 EST


On Fri 09-12-16 15:28:20, Michal Hocko wrote:
> Hi Andy,
> I am hitting the following
> [ 570.715345] BUG: using smp_processor_id() in preemptible [00000000] code: umount/6193
> [ 570.716880] caller is debug_smp_processor_id+0x17/0x19
> [ 570.717876] CPU: 2 PID: 6193 Comm: umount Tainted: G W 4.9.0-rc8-nofstest4-next-20161209-00012-g0db618dfb2cf #1017
> [ 570.720162] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-20161025_171302-gandalf 04/01/2014
> [ 570.720862] Call Trace:
> [ 570.720862] dump_stack+0x68/0x92
> [ 570.720862] check_preemption_disabled+0xce/0xe0
> [ 570.720862] debug_smp_processor_id+0x17/0x19
> [ 570.720862] vfree_atomic+0x2f/0x65
> [ 570.720862] put_task_stack+0xd0/0x13c
> [ 570.720862] kthread_stop+0x108/0x29a
> [ 570.720862] destroy_workqueue+0x167/0x1fb
> [ 570.720862] ext4_put_super+0x44/0x2fa
> [ 570.720862] generic_shutdown_super+0x6a/0xeb
> [ 570.720862] kill_block_super+0x27/0x67
> [ 570.720862] deactivate_locked_super+0x30/0x68
> [ 570.720862] deactivate_super+0x3e/0x41
> [ 570.720862] cleanup_mnt+0x58/0x76
> [ 570.720862] __cleanup_mnt+0x12/0x14
> [ 570.720862] task_work_run+0x77/0xa0
> [ 570.720862] exit_to_usermode_loop+0x67/0x92
> [ 570.720862] do_syscall_64+0x16c/0x197
> [ 570.720862] entry_SYSCALL64_slow_path+0x25/0x25
>
> it seems that vfre_atomic doesn't like it has preemption enabled in
> __vfree_deferred which calls this_cpu_ptr. I haven't studied the code
> deeply but I guess what we want is to put vfree_atomic into the atomic
> context.

OK, so I've double checked the code and I believe that the warning is
harmless. __vfree_deferred is indeed called from the preemptible context
but the current implementation should be safe wrt. preemption. llist_add
should work just fine even when preempted and never produce a corrupted
list AFAIU. schedule_work should be OK as well.

> Something like the following or at least disable preemption
> around vfree_atomic.

Scratch that. If anything this should be handled in __vfree_deferred
rather than hidden in the caller.

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 43f0608dddc6..581fe682e8d7 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1498,10 +1498,11 @@ static void __vunmap(const void *addr, int deallocate_pages)

static inline void __vfree_deferred(const void *addr)
{
- struct vfree_deferred *p = this_cpu_ptr(&vfree_deferred);
+ struct vfree_deferred *p = &get_cpu_var(vfree_deferred);

if (llist_add((struct llist_node *)addr, &p->list))
schedule_work(&p->wq);
+ put_cpu_var(vfree_deferred);
}

/**

but maybe this is just too prudent and we should simply do:

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 43f0608dddc6..5ecd32258d6a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1498,7 +1498,11 @@ static void __vunmap(const void *addr, int deallocate_pages)

static inline void __vfree_deferred(const void *addr)
{
- struct vfree_deferred *p = this_cpu_ptr(&vfree_deferred);
+ /*
+ * llist_add is preemption safe so we do not have to care
+ * about preemption being disabled here
+ */
+ struct vfree_deferred *p = raw_cpu_ptr(&vfree_deferred);

if (llist_add((struct llist_node *)addr, &p->list))
schedule_work(&p->wq);

--
Michal Hocko
SUSE Labs