netdev - Re: [PATCH] b44: add 64 bit stats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABF+-6XVCTDZedzcp-gWACYpG9NLmZUdwW2zsXC8WNKWk3dE5w@mail.gmail.com>
Date:	Thu, 19 Jul 2012 21:56:30 -0400
From:	Kevin Groeneveld <kgroeneveld@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] b44: add 64 bit stats

Hi Eric,

On Sun, Jul 15, 2012 at 3:26 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> So you must use u64_stats_fetch_begin_bh()
>
> Because on 32bit, uniprocessor, u64_stats_fetch_begin() only disables
> preemption. (there is no seqlock in syncp)
>
> So softirq are allowed to interrupt you and corrupt your stats while you
> read them, and you dont notice you have to retry.

I am still trying to make sure I understand this fully.  I want to
update some other drivers with 64 bit stats as well.  What you said
seems to make sense, but...

I was looking at the virtio_net.c driver.  One spot in this driver
which updates the stats is the receive_buf function.  recive_buf is
called from virtnet_poll which is registered as a napi poll function.
According to Documentation/networking/netdevices.txt the poll function
is called in a softirq context.  However, the function which reads the
stats uses u64_stats_fetch_begin/u64_stats_fetch_retry.  Shouldn't
this be u64_stats_fetch_begin_bh/u64_stats_fetch_retry_bh for the
exact reasons you described for my b44 patch?

Kevin
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html