Tomonari Horikoshi
2007-Jan-25 11:04 UTC
[Xen-ia64-devel] [PATCH][RFC] correction trouble of short message in UDP
Hi all. I modified the patch to the following trouble. "When I executed "netperf" by a short message of UDP, PV domain issued Call trace." This error occurs because earliness grant is filled from the queue check of tx_slot when a lot of message is sent. I stopped "Add netfornt tx_queue_len" being used, and I was added to check gnttab_empty_grant_references() in netfront_tx_slot_available(). I confirmed the not error because of this correction. Is there a better correction? Best regards. Tomonari Horikoshi, Tomonari Horikoshi wrote:---------------------- Sent: Thu, 25 Jan 2007 13:44:30 +0900 Subject: Re: [Xen-devel] [PATCH] Add netfornt tx_queue_len> Hi Herbert-san > > Thank you for your comment. > > I agreed. > I examine the another way. > > It is likely to go well if something is added to the check > on netfront_tx_slot_available(). > > Best regards. > > Tomonari Horikoshi, > > Herbert Xu wrote:---------------------- > Sent: Wed, 24 Jan 2007 13:29:51 +1100 > Subject: Re: [Xen-devel] [PATCH] Add netfornt tx_queue_len > > > On Wed, Jan 24, 2007 at 01:37:55AM +0000, Tomonari Horikoshi wrote: > > > > > > When I executed "netperf" by a short message of UDP, > > > PV domain and PV-on-HVM driver issued Call trace. > > > > > > I think that GrantEntry was filled with a lot of messages processings. > > > > > > This problem is generated in IA64 only. > > > Probably, I think that I am the following problems. > > > > > > In IA64 > > > NET_TX_RING_SIZE 1024, NR_GRANT_ENTRIES 2048 > > > In x86 > > > NET_TX_RING_SIZE 256, NR_GRANT_ENTRIES 2048 > > > > > > I corrected to check "number of unprocessing queue > tx_queue_len" before Grant was filled. > > > > > > However, my correction influences x86. > > > Please teach to me in that when there is a better improvement. > > > > Sorry, but this patch looks bogus. The tx queue is maintained by > > Linux and has nothing to do with the driver. So limiting its length > > based on internal state of the driver can''t be right. > > > > We need to find out what''s really going wrong with the grant table > > entries here. > > > > Cheers,_______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@lists.xensource.com http://lists.xensource.com/xen-ia64-devel
Keir Fraser
2007-Jan-25 12:26 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
On 25/1/07 11:04, "Tomonari Horikoshi" <t.horikoshi@jp.fujitsu.com> wrote:> I confirmed the not error because of this correction. > Is there a better correction?It''s along the right lines so I check it in as is. However, you probably need to check that you have enough grant entries for worst-case packet fragmentation (a packet may require more than one grant). Or start_xmit() needs to be able to buffer one packet until it has enough grants to proceed with its transmission. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Herbert Xu
2007-Jan-25 13:40 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
Keir Fraser <keir@xensource.com> wrote:> On 25/1/07 11:04, "Tomonari Horikoshi" <t.horikoshi@jp.fujitsu.com> wrote: > >> I confirmed the not error because of this correction. >> Is there a better correction? > > It''s along the right lines so I check it in as is. However, you probably > need to check that you have enough grant entries for worst-case packet > fragmentation (a packet may require more than one grant). Or start_xmit() > needs to be able to buffer one packet until it has enough grants to proceed > with its transmission.How about making the total number of grant table entries equal the number of TX slots? That way we won''t have to check anything at all. It also doesn''t make sense to have more TX slots than grant table entries. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jan-25 14:03 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
On 25/1/07 13:40, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:> How about making the total number of grant table entries equal the > number of TX slots? That way we won''t have to check anything at all. > > It also doesn''t make sense to have more TX slots than grant table entries.Yes, this is a good idea! So we should replace current netfront_tx_slot_available implementation with: (tx->req_prod_pvt - tx->rsp_cons) < (TX_MAX_TARGET - MAX_SKB_FRAGS - 2) ?? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jan-25 14:06 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
On 25/1/07 14:03, "Keir Fraser" <keir@xensource.com> wrote:>> How about making the total number of grant table entries equal the >> number of TX slots? That way we won''t have to check anything at all. >> >> It also doesn''t make sense to have more TX slots than grant table entries. > > Yes, this is a good idea! > > So we should replace current netfront_tx_slot_available implementation with: > (tx->req_prod_pvt - tx->rsp_cons) < (TX_MAX_TARGET - MAX_SKB_FRAGS - 2) > > ??Sorry, what I''m suggesting actually isn''t the same as your proposal. :-) I think mine would work though, and 256 grants should be plenty as a maximum. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Herbert Xu
2007-Jan-25 14:17 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
On Thu, Jan 25, 2007 at 02:03:19PM +0000, Keir Fraser wrote:> > So we should replace current netfront_tx_slot_available implementation with: > (tx->req_prod_pvt - tx->rsp_cons) < (TX_MAX_TARGET - MAX_SKB_FRAGS - 2)Yeah that''s the idea. I was thinking of something like this: diff -r bea505a69722 linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Tue Jan 23 15:58:05 2007 +0000 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Fri Jan 26 01:14:19 2007 +1100 @@ -138,8 +138,10 @@ static inline int netif_needs_gso(struct #define GRANT_INVALID_REF 0 -#define NET_TX_RING_SIZE __RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE) -#define NET_RX_RING_SIZE __RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE) +#define NET_TX_RING_SIZE \ + (__RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE) & 255) +#define NET_RX_RING_SIZE \ + (__RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE) & 255) struct netfront_info { struct list_head list; @@ -159,7 +161,7 @@ struct netfront_info { /* Receive-ring batched refills. */ #define RX_MIN_TARGET 8 #define RX_DFL_MIN_TARGET 64 -#define RX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256) +#define RX_MAX_TARGET NET_RX_RING_SIZE unsigned rx_min_target, rx_max_target, rx_target; struct sk_buff_head rx_batch; @@ -172,7 +174,7 @@ struct netfront_info { struct sk_buff *tx_skbs[NET_TX_RING_SIZE+1]; struct sk_buff *rx_skbs[NET_RX_RING_SIZE]; -#define TX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256) +#define TX_MAX_TARGET NET_TX_RING_SIZE grant_ref_t gref_tx_head; grant_ref_t grant_tx_ref[NET_TX_RING_SIZE + 1]; grant_ref_t gref_rx_head; Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Herbert Xu
2007-Jan-25 14:21 UTC
Re: [Xen-devel] [PATCH][RFC] correction trouble of short message in UDP
On Fri, Jan 26, 2007 at 01:17:19AM +1100, Herbert Xu wrote:> > diff -r bea505a69722 linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c > --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Tue Jan 23 15:58:05 2007 +0000 > +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Fri Jan 26 01:14:19 2007 +1100 > @@ -138,8 +138,10 @@ static inline int netif_needs_gso(struct > > #define GRANT_INVALID_REF 0 > > -#define NET_TX_RING_SIZE __RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE) > -#define NET_RX_RING_SIZE __RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE) > +#define NET_TX_RING_SIZE \ > + (__RING_SIZE((struct netif_tx_sring *)0, PAGE_SIZE) & 255) > +#define NET_RX_RING_SIZE \ > + (__RING_SIZE((struct netif_rx_sring *)0, PAGE_SIZE) & 255)Actually this can''t work because the size is independently derived in two different places... So your idea definitely sounds better. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel