Re: [PATCH] virtio-net: fix a race on 32bit arches
|[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]|
On 06/06/2012 04:45 PM, Eric Dumazet wrote:
On Wed, 2012-06-06 at 10:35 +0200, Eric Dumazet wrote:From: Eric Dumazet<edumazet@xxxxxxxxxx> commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race on 32bit arches. We must use separate syncp for rx and tx path as they can be run at the same time on different cpus. Thus one sequence increment can be lost and readers spin forever. Signed-off-by: Eric Dumazet<edumazet@xxxxxxxxxx> Cc: Stephen Hemminger<shemminger@xxxxxxxxxx> Cc: Michael S. Tsirkin<mst@xxxxxxxxxx> Cc: Jason Wang<jasowang@xxxxxxxxxx> ---Just to make clear : even using percpu stats/syncp, we have no guarantee that write_seqcount_begin() is done with one instruction.  It is OK on x86 if "incl" instruction is generated by the compiler, but on a RISC cpu, the "load memory,%reg ; inc %reg ; store %reg,memory" can be interrupted. So if you are 100% sure all paths are safe against preemption/BH, then this patch is not needed, but a big comment in the code would avoid adding possible races in the future.
Thanks for explaing, current virtio-net is safe I think. But the patch is still needed as my patch would update the statistics in irq.
 If done with one instruction, we still have a race, since a reader might see an even sequence and conclude no writer is inside the critical section. So read values could be wrong. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization