* Re: [PATCH] tg3_msi() and weakly ordered memory [not found] <B1508D50A0692F42B217C22C02D84972067F0805@NT-IRVA-0741.brcm.ad.broadcom.com> @ 2005-06-14 15:46 ` Grant Grundler [not found] ` <1118771563.7059.30.camel@rh4> 0 siblings, 1 reply; 4+ messages in thread From: Grant Grundler @ 2005-06-14 15:46 UTC (permalink / raw) To: Michael Chan; +Cc: David S. Miller, iod00d, netdev On Mon, Jun 13, 2005 at 11:54:23PM -0700, Michael Chan wrote: > > Once you write "0x1" to the mailbox register, the device stops > > updating the status block and stops generating interrupts. > > > > That is what makes a lot of things safe. > > Only interrupts are stopped, status block will still be updated subject to > during-ints coalescing. Will setting during-ints to a very high threshhold essentially allow us to "indefinitely" process stuff without taking any interrupts? Would the threshhold counter get reset every time we write back the status tag WITHOUT re-enableing interrupts? If not, I suspect the CPU will circulate in tg3_poll until during-ints is exhausted and DMA will stop until CPU reenables interrupts. ie not until it's done processing outstanding packets. thanks, grant ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <1118771563.7059.30.camel@rh4>]
[parent not found: <20050614211530.GB25516@esmail.cup.hp.com>]
* Re: [PATCH] tg3_msi() and weakly ordered memory [not found] ` <20050614211530.GB25516@esmail.cup.hp.com> @ 2005-06-21 23:56 ` David S. Miller 2005-06-22 5:20 ` Grant Grundler 2005-06-22 12:56 ` [PATCH] dont use strlen() but the result from a prior sprintf() Eric Dumazet 0 siblings, 2 replies; 4+ messages in thread From: David S. Miller @ 2005-06-21 23:56 UTC (permalink / raw) To: iod00d; +Cc: mchan, netdev Ok, here is the patch I came up with as a result of this thread. Michael stated he would investigate using a pure tag comparison in place of tg3_has_work() when the chip is using tagged interrupts. Thanks. [TG3]: Fix missing memory barriers and SD_STATUS_UPDATED bit clearing. There must be a rmb() between reading the status block tag and calling tg3_has_work(). This was missing in tg3_mis() and tg3_interrupt_tagged(). tg3_poll() got it right. Also, SD_STATUS_UPDATED must be cleared in the status block right before we call tg3_has_work(). Only tg3_poll() got this wrong. Based upon patches and commentary from Grant Grundler and Michael Chan. Signed-off-by: David S. Miller <davem@davemloft.net> --- 1/drivers/net/tg3.c.~1~ 2005-06-21 16:39:19.000000000 -0700 +++ 2/drivers/net/tg3.c 2005-06-21 16:47:55.000000000 -0700 @@ -2929,6 +2929,7 @@ if (tp->tg3_flags & TG3_FLAG_TAGGED_STATUS) tp->last_tag = sblk->status_tag; rmb(); + sblk->status &= ~SD_STATUS_UPDATED; /* if no more work, tell net stack and NIC we're done */ done = !tg3_has_work(tp); @@ -2964,6 +2965,7 @@ */ tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001); tp->last_tag = sblk->status_tag; + rmb(); sblk->status &= ~SD_STATUS_UPDATED; if (likely(tg3_has_work(tp))) netif_rx_schedule(dev); /* schedule NAPI poll */ @@ -3051,6 +3053,7 @@ tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001); tp->last_tag = sblk->status_tag; + rmb(); sblk->status &= ~SD_STATUS_UPDATED; if (likely(tg3_has_work(tp))) netif_rx_schedule(dev); /* schedule NAPI poll */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tg3_msi() and weakly ordered memory 2005-06-21 23:56 ` David S. Miller @ 2005-06-22 5:20 ` Grant Grundler 2005-06-22 12:56 ` [PATCH] dont use strlen() but the result from a prior sprintf() Eric Dumazet 1 sibling, 0 replies; 4+ messages in thread From: Grant Grundler @ 2005-06-22 5:20 UTC (permalink / raw) To: David S. Miller; +Cc: iod00d, mchan, netdev On Tue, Jun 21, 2005 at 04:56:34PM -0700, David S. Miller wrote: > > Ok, here is the patch I came up with as a result of this thread. looks good to me. > Michael stated he would investigate using a pure tag comparison in > place of tg3_has_work() when the chip is using tagged interrupts. The more I think about it, the more I like the idea of each ISR calling into a different tg3_poll routine. The specific _poll() routine could do the "is there more work" checking instead the TX/RX ring cleanup code. The main reason is the "more work" checks can be better optimized for MSI (use tags) vs IRQ Line interrupt (use ring indices) handlers. I also hope to reduce cacheline movement by touching the status block fewer times. This isn't a trivial patch and I'm short on time (preparing stuff for OLS and HP World before my vacation). If there is still interest, I can prototype a patch in late August or Sept (about 8 weeks from now). > Thanks. Welcome and thanks too. BTW, I greatly appreciate Michael clarifying tg3 behavior. thanks, grant ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] dont use strlen() but the result from a prior sprintf() 2005-06-21 23:56 ` David S. Miller 2005-06-22 5:20 ` Grant Grundler @ 2005-06-22 12:56 ` Eric Dumazet 1 sibling, 0 replies; 4+ messages in thread From: Eric Dumazet @ 2005-06-22 12:56 UTC (permalink / raw) To: David S. Miller; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 769 bytes --] Hi David Small patch to save an unecessary call to strlen() : sprintf() gave us the length, just trust it. Thank you Eric Dumazet diff -Nu linux-2.6.12-orig/net/socket.c linux-2.6.12/net/socket.c --- linux-2.6.12-orig/net/socket.c 2005-06-22 14:47:56.000000000 +0200 +++ linux-2.6.12/net/socket.c 2005-06-22 14:49:22.000000000 +0200 @@ -382,9 +382,8 @@ goto out; } - sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino); + this.len = sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino); this.name = name; - this.len = strlen(name); this.hash = SOCK_INODE(sock)->i_ino; file->f_dentry = d_alloc(sock_mnt->mnt_sb->s_root, &this); [-- Attachment #2: patch.1 --] [-- Type: text/plain, Size: 446 bytes --] --- linux-2.6.12-orig/net/socket.c 2005-06-22 14:47:56.000000000 +0200 +++ linux-2.6.12/net/socket.c 2005-06-22 14:49:22.000000000 +0200 @@ -382,9 +382,8 @@ goto out; } - sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino); + this.len = sprintf(name, "[%lu]", SOCK_INODE(sock)->i_ino); this.name = name; - this.len = strlen(name); this.hash = SOCK_INODE(sock)->i_ino; file->f_dentry = d_alloc(sock_mnt->mnt_sb->s_root, &this); ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-06-22 12:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <B1508D50A0692F42B217C22C02D84972067F0805@NT-IRVA-0741.brcm.ad.broadcom.com>
2005-06-14 15:46 ` [PATCH] tg3_msi() and weakly ordered memory Grant Grundler
[not found] ` <1118771563.7059.30.camel@rh4>
[not found] ` <20050614211530.GB25516@esmail.cup.hp.com>
2005-06-21 23:56 ` David S. Miller
2005-06-22 5:20 ` Grant Grundler
2005-06-22 12:56 ` [PATCH] dont use strlen() but the result from a prior sprintf() Eric Dumazet
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.