2003-06-16 01:17:23

by Jeff Sipek

[permalink] [raw]
Subject: 64-bit fields in struct net_device_stats

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello all,
I was looking for a project that would take some time to finish, and I think
I found it - converting all code in the kernel to use u_int64_t (or similar)
instead of unsigned long in struct net_device_stats.
Now, I have an idea on my mind about how to do this:

I'd move the structure to a new file in linux/include/asm
(linux/include/asm-{arch}/netdevice.h, for example) and implement there
couple of functions that would change the counters in the structure
(something like: static inline void net_stats_txbytes_add(struct
net_device_stats* stats, unsigned long len)). These would lock (if necessary
- - 32-bit architectures), add, unlock (if necessary.) The only thing is, that
all the NIC drivers in the kernel up to date would have to be changed to use
this new interface.

Now, my question: Is there a better way of accomplishing this?

Thanks,
Jeff.

- --
A CRAY is the only computer that runs an endless loop in just 4 hours...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE+7R3XwFP0+seVj/4RAolgAJ9QE4eLfKqrVhR9tktoZCHjcfarfgCcDb1A
HELhBfYleUbTSaTymmTsRJM=
=xu4t
-----END PGP SIGNATURE-----


2003-06-16 02:39:50

by Jeff Sipek

[permalink] [raw]
Subject: Re: 64-bit fields in struct net_device_stats

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sunday 15 June 2003 21:30, Jeff wrote:
<snip>
> These would lock (if
> necessary - 32-bit architectures), add, unlock (if necessary.)
<snip>

I now realize, that locking is out of the question. Also, it has been
suggested to use per cpu stats and overflow into a global counter. (Thanks
Zwane) This might be a better idea - the problem is, the counter won't be
100% accurate at all times. The degree of inaccuracy will vary with the
threshold value. On the other hand, if the threshold is relatively low, no
one will notice the difference these days.

Jeff.

- --
Please avoid sending me Word or PowerPoint attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE+7TEswFP0+seVj/4RAidcAJ0UAgHtRK4F1HHp8vOSLVOc5tdtRgCfXTyN
MA5sBYybdjJxwxAwtiUnE8I=
=MbW+
-----END PGP SIGNATURE-----

2003-06-16 03:09:47

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 64-bit fields in struct net_device_stats

On Sun, 15 Jun 2003, Jeff wrote:

> I now realize, that locking is out of the question. Also, it has been

Well spinlocks in particular would be particularly ugly here and cause
horrid cache line ping pong. Other methods of synchronization would have
to be looked at.

> suggested to use per cpu stats and overflow into a global counter. (Thanks
> Zwane) This might be a better idea - the problem is, the counter won't be
> 100% accurate at all times. The degree of inaccuracy will vary with the
> threshold value. On the other hand, if the threshold is relatively low, no
> one will notice the difference these days.

This would be one method of doing updates and for stats it would be fine,
however feel free to look into other ways...

Zwane
--
function.linuxpower.ca

2003-06-16 05:13:33

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 64-bit fields in struct net_device_stats

On Sun, 15 Jun 2003, Zwane Mwaikambo wrote:

> On Sun, 15 Jun 2003, Jeff wrote:
>
> > I now realize, that locking is out of the question. Also, it has been
>
> Well spinlocks in particular would be particularly ugly here and cause
> horrid cache line ping pong. Other methods of synchronization would have
> to be looked at.
>
> > suggested to use per cpu stats and overflow into a global counter. (Thanks
> > Zwane) This might be a better idea - the problem is, the counter won't be
> > 100% accurate at all times. The degree of inaccuracy will vary with the
> > threshold value. On the other hand, if the threshold is relatively low, no
> > one will notice the difference these days.
>
> This would be one method of doing updates and for stats it would be fine,
> however feel free to look into other ways...

Looking closer i'd say go for it, there isn't a real locking issue, if
you're looking for something to do this should be rather fun with not
that much breakage, of course you'd also have to update some userland
tools/utilities.

have fun,
Zwane
--
function.linuxpower.ca

2003-06-16 05:34:58

by Lincoln Dale

[permalink] [raw]
Subject: Re: 64-bit fields in struct net_device_stats

At 11:12 PM 15/06/2003 -0400, Zwane Mwaikambo wrote:
> > suggested to use per cpu stats and overflow into a global counter. (Thanks
> > Zwane) This might be a better idea - the problem is, the counter won't be
> > 100% accurate at all times. The degree of inaccuracy will vary with the
> > threshold value. On the other hand, if the threshold is relatively low, no
> > one will notice the difference these days.
>
>This would be one method of doing updates and for stats it would be fine,
>however feel free to look into other ways...

why not a set of counters which are toggled between.

e.g.
struct netdevice... {
uint64 tx_pkts_counter[2];
uint64 tx_octets_counter[2];
uint64 rx_pkts_counter[2];
uint64 rx_octets_counter[2];
int counter_bounce;
...
}


where readers simply do something like:
tx_bytes +=
netdevice[foo]->tx_octets_counter[(netdevice[foo]->counter_bounce)];

and writer(s) alternate between updating one uint64 and an alternate one?

in this manner, you don't need to do any special synchronization or atomic
updates or have any dependency on any particular architecture being able to
do atomic 64-bit ops.


cheers,

lincoln.

2003-06-16 13:49:06

by Jeff Sipek

[permalink] [raw]
Subject: Re: 64-bit fields in struct net_device_stats

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Monday 16 June 2003 01:47, Lincoln Dale wrote:
> why not a set of counters which are toggled between.
>
> e.g.
> struct netdevice... {
> uint64 tx_pkts_counter[2];
> uint64 tx_octets_counter[2];
> uint64 rx_pkts_counter[2];
> uint64 rx_octets_counter[2];
> int counter_bounce;
> ...
> }
<snip>

Hmm, interesting idea. The one thing is: There are 23 fields in struct
net_device_stats right now; if each of them is 64bits (=8 bytes), the whole
structure would be 184 bytes (with u_int64.) Now if I have two of each, the
size would grow to 368 bytes (plus the 4 bytes for counter_bounce). Now, the
question: would it be acceptable to have something that size?

Jeff.

- --
*NOTE: This message is ROT-13 encrypted twice for extra protection*
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.2 (GNU/Linux)

iD8DBQE+7c4FwFP0+seVj/4RAtGxAJsE+YY48y1mp+Y90FFH2Jz+hmU+fgCeK+gj
6ksJ8KxDYVo7rwxODwSAeT8=
=Sxo5
-----END PGP SIGNATURE-----