netdev - Re: [net 1/6] net/mlx5e: Use a spinlock to synchronize statistics

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]

Date:   Fri, 12 May 2017 12:13:54 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     saeedm@...lanox.com
Cc:     netdev@...r.kernel.org, galp@...lanox.com, kernel-team@...com
Subject: Re: [net 1/6] net/mlx5e: Use a spinlock to synchronize statistics

From: Saeed Mahameed <saeedm@...lanox.com>
Date: Fri, 12 May 2017 14:56:45 +0300

> From: Gal Pressman <galp@...lanox.com>
> 
> Add a spinlock to prevent races when querying statistics, for example
> querying counters in the middle of a non atomic memcpy() operation in
> mlx5e_update_stats().
> 
> This RW lock should be held when accessing priv->stats, to prevent other
> reads/writes.
> 
> Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
> Signed-off-by: Gal Pressman <galp@...lanox.com>
> Suggested-by: Eric Dumazet <eric.dumazet@...il.com>
> Cc: kernel-team@...com
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>

This is overkill, and that rwlock is going to show up in perf for some
workloads.

Furthermore, two kzalloc()'s for a single state update operation?
That's not reasonable either.

Use a seqlock, which is the primitive for handling this kind of
situation cheaply, and adds no atomics to the read path.

Thank you.