padata: avoid race in reordering
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@zx2c4.com> Acked-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This commit is contained in:
parent
74d1cf4897
commit
de5540d088
|
@ -186,19 +186,20 @@ static struct padata_priv *padata_get_next(struct parallel_data *pd)
|
||||||
|
|
||||||
reorder = &next_queue->reorder;
|
reorder = &next_queue->reorder;
|
||||||
|
|
||||||
|
spin_lock(&reorder->lock);
|
||||||
if (!list_empty(&reorder->list)) {
|
if (!list_empty(&reorder->list)) {
|
||||||
padata = list_entry(reorder->list.next,
|
padata = list_entry(reorder->list.next,
|
||||||
struct padata_priv, list);
|
struct padata_priv, list);
|
||||||
|
|
||||||
spin_lock(&reorder->lock);
|
|
||||||
list_del_init(&padata->list);
|
list_del_init(&padata->list);
|
||||||
atomic_dec(&pd->reorder_objects);
|
atomic_dec(&pd->reorder_objects);
|
||||||
spin_unlock(&reorder->lock);
|
|
||||||
|
|
||||||
pd->processed++;
|
pd->processed++;
|
||||||
|
|
||||||
|
spin_unlock(&reorder->lock);
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
spin_unlock(&reorder->lock);
|
||||||
|
|
||||||
if (__this_cpu_read(pd->pqueue->cpu_index) == next_queue->cpu_index) {
|
if (__this_cpu_read(pd->pqueue->cpu_index) == next_queue->cpu_index) {
|
||||||
padata = ERR_PTR(-ENODATA);
|
padata = ERR_PTR(-ENODATA);
|
||||||
|
|
Loading…
Reference in a new issue