On 2018/10/19 上午7:23, David Miller wrote:
From: Sebastian Andrzej Siewior <bige...@linutronix.de>
Date: Thu, 18 Oct 2018 10:43:13 +0200

on 32bit, lockdep notices that virtnet_open() and refill_work() invoke
try_fill_recv() from process context while virtnet_receive() invokes the
same function from BH context. The problem that the seqcounter within
u64_stats_update_begin() may deadlock if it is interrupted by BH and
then acquired again.

Introduce u64_stats_update_begin_bh() which disables BH on 32bit
architectures. Since the BH might interrupt the worker, this new
function should not limited to SMP like the others which are expected
to be used in softirq.

With this change we might lose increments but this is okay. The
important part that the two 32bit parts of the 64bit counter are not
corrupted.

Fixes: 461f03dc99cf6 ("virtio_net: Add kick stats").
Suggested-by: Stephen Hemminger <step...@networkplumber.org>
Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
Trying to get down to the bottom of this:

1) virtnet_receive() runs from softirq but only if NAPI is active and
    enabled.  It is in this context that it invokes try_fill_recv().

2) refill_work() runs from process context, but disables NAPI (and
    thus invocation of virtnet_receive()) before calling
    try_fill_recv().

3) virtnet_open() invokes from process context as well, but before the
    NAPI instances are enabled, it is same as case #2.

4) virtnet_restore_up() is the same situations as #3.

Therefore I agree that this is a false positive, and simply lockdep
cannot see the NAPI synchronization done by case #2.

I think we shouldn't add unnecessary BH disabling here, and instead
find some way to annotate this for lockdep's sake.

Thank you.


+1

Thanks

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to