On Tue, Apr 04, 2017 at 01:53:15PM +0200, Jason A. Donenfeld wrote: > Herbert applied this to his tree. It's probably a good stable > candidate, since it's a two line change to fix a race condition. > > On Fri, Mar 24, 2017 at 3:16 PM, Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > Jason A. Donenfeld <Jason@xxxxxxxxx> wrote: > >> Under extremely heavy uses of padata, crashes occur, and with list > >> debugging turned on, this happens instead: > >> > >> [87487.298728] WARNING: CPU: 1 PID: 882 at lib/list_debug.c:33 > >> __list_add+0xae/0x130 > >> [87487.301868] list_add corruption. prev->next should be next > >> (ffffb17abfc043d0), but was ffff8dba70872c80. (prev=ffff8dba70872b00). > >> [87487.339011] [<ffffffff9a53d075>] dump_stack+0x68/0xa3 > >> [87487.342198] [<ffffffff99e119a1>] ? console_unlock+0x281/0x6d0 > >> [87487.345364] [<ffffffff99d6b91f>] __warn+0xff/0x140 > >> [87487.348513] [<ffffffff99d6b9aa>] warn_slowpath_fmt+0x4a/0x50 > >> [87487.351659] [<ffffffff9a58b5de>] __list_add+0xae/0x130 > >> [87487.354772] [<ffffffff9add5094>] ? _raw_spin_lock+0x64/0x70 > >> [87487.357915] [<ffffffff99eefd66>] padata_reorder+0x1e6/0x420 > >> [87487.361084] [<ffffffff99ef0055>] padata_do_serial+0xa5/0x120 > >> > >> padata_reorder calls list_add_tail with the list to which its adding > >> locked, which seems correct: > >> > >> spin_lock(&squeue->serial.lock); > >> list_add_tail(&padata->list, &squeue->serial.list); > >> spin_unlock(&squeue->serial.lock); > >> > >> This therefore leaves only place where such inconsistency could occur: > >> if padata->list is added at the same time on two different threads. > >> This pdata pointer comes from the function call to > >> padata_get_next(pd), which has in it the following block: > >> > >> next_queue = per_cpu_ptr(pd->pqueue, cpu); > >> padata = NULL; > >> reorder = &next_queue->reorder; > >> if (!list_empty(&reorder->list)) { > >> padata = list_entry(reorder->list.next, > >> struct padata_priv, list); > >> spin_lock(&reorder->lock); > >> list_del_init(&padata->list); > >> atomic_dec(&pd->reorder_objects); > >> spin_unlock(&reorder->lock); > >> > >> pd->processed++; > >> > >> goto out; > >> } > >> out: > >> return padata; > >> > >> I strongly suspect that the problem here is that two threads can race > >> on reorder list. Even though the deletion is locked, call to > >> list_entry is not locked, which means it's feasible that two threads > >> pick up the same padata object and subsequently call list_add_tail on > >> them at the same time. The fix is thus be hoist that lock outside of > >> that block. > >> > >> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx> > > > > Patch applied. Thanks. Any clue as to what the git commit id is? thanks, greg k-h