All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefani Seibold <stefani@seibold.net>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	davem@davemloft.net, netdev@vger.kernel.org
Subject: Re: [PATCH] UDPCP Communication Protocol
Date: Fri, 31 Dec 2010 11:29:13 +0100	[thread overview]
Message-ID: <1293791353.5193.1.camel@wall-e> (raw)
In-Reply-To: <1293790501.2973.33.camel@edumazet-laptop>

Am Freitag, den 31.12.2010, 11:15 +0100 schrieb Eric Dumazet:
> Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> écrit :
> > +				spin_lock_irqsave(&spinlock, flags);
> > +				udpcp_stat.txMsgs++;
> > +				spin_unlock_irqrestore(&spinlock, flags);
> 
> This is really ugly for different reasons :
> 
> 1) Naming a lock, even static "spinlock" is ugly.

Agree...

> 2) Using a lock for stats is not necessary, and
>    disabling hard irqs is not necessary either (spinlock_bh() would be
> more than enough)
>   
>    At a very minimum, you should use atomic_t so that no lock is needed
> 
> 3) Network stack widely use MIB per_cpu counters.
>  As you use UDP, you could take a look at UDP_INC_STATS_BH()/
> UDP_INC_STATS_USER() implementation for an example.
> 

I will have look at this and revamp it.



  reply	other threads:[~2010-12-31 10:28 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-31  9:29 [PATCH] UDPCP Communication Protocol stefani
2010-12-31 10:00 ` Eric Dumazet
2010-12-31 10:22   ` Stefani Seibold
2010-12-31 10:41     ` Eric Dumazet
2010-12-31 11:23       ` Stefani Seibold
2010-12-31 11:54         ` Eric Dumazet
2011-01-01 21:40           ` Stefani Seibold
2011-01-10 22:28             ` Hagen Paul Pfeifer
2010-12-31 10:15 ` Eric Dumazet
2010-12-31 10:29   ` Stefani Seibold [this message]
2010-12-31 10:35 ` Eric Dumazet
2010-12-31 11:25 ` Eric Dumazet
2010-12-31 12:00   ` Eric Dumazet
2011-01-01 21:28     ` Stefani Seibold
2010-12-31 17:46 ` Stephen Hemminger
2010-12-31 17:46 ` Stephen Hemminger
2011-01-06 20:11 ` Pavel Machek
2011-01-06 20:17   ` David Miller
2011-01-10 22:53 ` Jesper Juhl
2011-01-11  0:49 ` Hagen Paul Pfeifer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1293791353.5193.1.camel@wall-e \
    --to=stefani@seibold.net \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.