netdev
[Top] [All Lists]

Re: [PATCH]snmp6 64-bit counter support in proc.c

To: "David S. Miller" <davem@xxxxxxxxxx>
Subject: Re: [PATCH]snmp6 64-bit counter support in proc.c
From: Krishna Kumar <kumarkr@xxxxxxxxxx>
Date: Wed, 28 Jan 2004 11:19:36 -0800
Cc: kuznet@xxxxxxxxxxxxx, mashirle@xxxxxxxxxx, netdev@xxxxxxxxxxx, Shirley Ma <xma@xxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx

Hi Dave,

> We can overflow the lower-half TWICE in the res1=...res2=... sequence
> you do, and this res2 reread in the if branch can overflow itself again,

The snippet assumed that the number will not overflow 4G times twice within
two reads, which I thought was quite a reasonable assumption.

I think a fast and clean solution is using Seq locks, which is how jiffies_64
is implemented on 32 bit systems. The writers always get the spin lock (can
be per cpu with zero contention/thrashing) and increment the 'seq' number of
seqlock data struct (all inside write_seqlock), then modify the 64 bit counter,
and release the lock which increases the 'seq' again. The readers don't get
a lock at all, they just read the 'seq' before and after the reading of the 64 bit
counter, and if it changed, it needs to repeat this cycle of reads. In this case,
no comparison of high/low order words are also needed. Does that sound
better ?

thanks,

- KK

Inactive hide details for "David S. Miller" <davem@xxxxxxxxxx>"David S. Miller" <davem@xxxxxxxxxx>




          "David S. Miller" <davem@xxxxxxxxxx>

          01/28/2004 11:09 AM



To: Krishna Kumar/Beaverton/IBM@IBMUS
cc: kuznet@xxxxxxxxxxxxx, mashirle@xxxxxxxxxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx, Shirley Ma/Beaverton/IBM@IBMUS
Subject: Re: [PATCH]snmp6 64-bit counter support in proc.c


On Thu, 22 Jan 2004 18:45:18 -0800
Krishna Kumar <kumarkr@xxxxxxxxxx> wrote:

> / * Overflow, sync and re-read, the next read is guaranteed to
> be greater
> * than res1.
> */
> synchronize_kernel();
> res2 = *((__u64 *) (((void *) per_cpu_ptr(mib[0], i)) + sizeof
> (__u64) * nr)));

I don't understand how your scheme can work.

We're trying to detect if the upper 32-bit half of the 64-bit value
belongs with the lower-half. We can overflow the lower-half TWICE
in the res1=...res2=... sequence you do, and this res2 reread in the if
branch can overflow itself again, combining a lower and upper half which
do not belong to each other.

This is not so trivial to fix, see? :)

GIF image

<Prev in Thread] Current Thread [Next in Thread>