Subject: Re: tbench regression in 2.6.25-rc1
From: "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>
To: Eric Dumazet <dada1@cosmosbay.com>
Cc: Valdis.Kletnieks@vt.edu, David Miller <davem@davemloft.net>,
       herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org,
       netdev@vger.kernel.org
In-Reply-To: <47BA87F0.1050709@cosmosbay.com>
References: <47B52B95.3070607@cosmosbay.com>
	 <1203057044.3027.134.camel@ymzhang> <47B59FFC.4030603@cosmosbay.com>
	 <20080215.152200.145584182.davem@davemloft.net>
	 <1203322358.3027.200.camel@ymzhang>
	 <20040.1203356033@turing-police.cc.vt.edu>
	 <1203403903.3248.29.camel@ymzhang>  <47BA87F0.1050709@cosmosbay.com>
Content-Type: text/plain; charset=utf-8
Date: Wed, 20 Feb 2008 15:04:41 +0800
Message-Id: <1203491081.3248.60.camel@ymzhang>
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4205
Lines: 114

On Tue, 2008-02-19 at 08:40 +0100, Eric Dumazet wrote:
> Zhang, Yanmin a ?crit :
> > On Mon, 2008-02-18 at 12:33 -0500, Valdis.Kletnieks@vt.edu wrote: 
> >> On Mon, 18 Feb 2008 16:12:38 +0800, "Zhang, Yanmin" said:
> >>
> >>> I also think __refcnt is the key. I did a new testing by adding 2 unsigned long
> >>> pading before lastuse, so the 3 members are moved to next cache line. The performance is
> >>> recovered.
> >>>
> >>> How about below patch? Almost all performance is recovered with the new patch.
> >>>
> >>> Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
> >> Could you add a comment someplace that says "refcnt wants to be on a different
> >> cache line from input/output/ops or performance tanks badly", to warn some
> >> future kernel hacker who starts adding new fields to the structure?
> > Ok. Below is the new patch.
> > 
> > 1) Move tclassid under ops in case CONFIG_NET_CLS_ROUTE=y. So sizeof(dst_entry)=200
> > no matter if CONFIG_NET_CLS_ROUTE=y/n. I tested many patches on my 16-core tigerton by
> > moving tclassid to different place. It looks like tclassid could also have impact on
> > performance.
> > If moving tclassid before metrics, or just don't move tclassid, the performance isn't
> > good. So I move it behind metrics.
> > 
> > 2) Add comments before __refcnt.
> > 
> > If CONFIG_NET_CLS_ROUTE=y, the result with below patch is about 18% better than
> > the one without the patch.
> > 
> > If CONFIG_NET_CLS_ROUTE=n, the result with below patch is about 30% better than
> > the one without the patch.
> > 
> > Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
> > 
> > ---
> > 
> > --- linux-2.6.25-rc1/include/net/dst.h	2008-02-21 14:33:43.000000000 +0800
> > +++ linux-2.6.25-rc1_work/include/net/dst.h	2008-02-22 12:52:19.000000000 +0800
> > @@ -52,15 +52,10 @@ struct dst_entry
> >  	unsigned short		header_len;	/* more space at head required */
> >  	unsigned short		trailer_len;	/* space to reserve at tail */
> >  
> > -	u32			metrics[RTAX_MAX];
> > -	struct dst_entry	*path;
> > -
> > -	unsigned long		rate_last;	/* rate limiting for ICMP */
> >  	unsigned int		rate_tokens;
> > +	unsigned long		rate_last;	/* rate limiting for ICMP */
> >  
> > -#ifdef CONFIG_NET_CLS_ROUTE
> > -	__u32			tclassid;
> > -#endif
> > +	struct dst_entry	*path;
> >  
> >  	struct neighbour	*neighbour;
> >  	struct hh_cache		*hh;
> > @@ -70,10 +65,20 @@ struct dst_entry
> >  	int			(*output)(struct sk_buff*);
> >  
> >  	struct  dst_ops	        *ops;
> > -		
> > -	unsigned long		lastuse;
> > +
> > +	u32			metrics[RTAX_MAX];
> > +
> > +#ifdef CONFIG_NET_CLS_ROUTE
> > +	__u32			tclassid;
> > +#endif
> > +
> > +	/*
> > +	 * __refcnt wants to be on a different cache line from
> > +	 * input/output/ops or performance tanks badly
> > +	 */
> >  	atomic_t		__refcnt;	/* client references	*/
> >  	int			__use;
> > +	unsigned long		lastuse;
> >  	union {
> >  		struct dst_entry *next;
> >  		struct rtable    *rt_next;
> > 
> > 
> > 
> 
> I prefer this patch, but unfortunatly your perf numbers are for 64 bits kernels.
> 
> Could you please test now with 32 bits one ?
I tested it with 32bit 2.6.25-rc1 on 8-core stoakley. The result almost has no difference
between pure kernel and patched kernel.

New update: On 8-core stoakley, the regression becomes 2~3% with kernel 2.6.25-rc2. On
tigerton, the regression is still 30% with 2.6.25-rc2. On Tulsa( 8 cores+hyperthreading),
the regression is still 4% with 2.6.25-rc2.

With my patch, on tigerton, almost all regression disappears. On tulsa, only about 2%
regression disappears.

So this issue is triggerred with multiple-cpu. Perhaps process scheduler is another
factor causing the issue to happen, but it's very hard to change scheduler.


Eric,

I tested your new patch in function loopback_xmit. It has no improvement, while it doesn't
introduce new issues. As you tested it on dual-core machine and got improvement, how about
merging your patch with mine?

-yanmin


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/