Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
If the register access function selection will truly be an 'if' test cascade, then I ACK these patches. Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
From: Jeff Garzik <[EMAIL PROTECTED]> Date: Thu, 28 Jul 2005 16:29:11 -0400 > If the register access function selection will truly be an 'if' test > cascade, then I ACK these patches. Yeah, it would. I'll more deeply review and integrate Michael's work later today. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
On Thu, Jul 28, 2005 at 10:33:12AM -0700, Michael Chan wrote: > If on the other hand, tg3 is loaded on different chips all using different > access > function pointers, and there is steady and equal amount of traffic going > through > all these chips, I suppose the call destinations will no longer be > predictable > most of the time. But in this scenario, I doubt that conditional branches > with > direct calls will fare any better. I concur w/ that assessment. Combine that w/ the potential ugliness of profligate expansion of if...else if...else if...else if...else style branches, and I think using the function pointers is acceptable and probably preferrable. John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
Jeff Garzik wrote: > In x86-based CPUs at least (the largest tg3 platform), branch > prediction > often prefers > > if (...) > direct_func_1() > else > direct_func_2() > > to > > tp->func() > The following is from Intel Pentium 4 and Intel Xeon ProcessorOptimization page 2-20: Indirect branches, such as switch statements, computed GOTOs or calls through pointers, can jump to an arbitrary number of locations. If the code sequence is such that the target destination of a branch goes to the same address most of the time, then the BTB will predict accurately most of the time. If, however, the target destination is not predictable, performance can degrade quickly. If tg3 is loaded on multiple chips using the same register access function pointers, the destinations of the indirect calls will be the same and the destinations should be predicted correctly by the BTB. If on the other hand, tg3 is loaded on different chips all using different access function pointers, and there is steady and equal amount of traffic going through all these chips, I suppose the call destinations will no longer be predictable most of the time. But in this scenario, I doubt that conditional branches with direct calls will fare any better. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
From: "Michael Chan" <[EMAIL PROTECTED]> Date: Wed, 27 Jul 2005 22:33:32 -0700 > But with so many different workaround methods > (TG3_FLAG_MBOX_WRITE_REORDER, TG3_FLAG_TXD_MBOX_HWBUG, > TG3_FLG2_ICH_WORKAROUND, TG3_FLAG_5701_REG_WRITE_BUG, etc), it's > more like: > > if (...) > direct_func_1() > else if (...) > direct_func_2() > else if (...) > direct_func_3() > else > direct_func_4() > > At some point I suspect the indirect function pointer method will > become better. That's a good point. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
Jeff Garzik wrote: > Is this theory, or it has been actually measured? > > In x86-based CPUs at least (the largest tg3 platform), branch > prediction > often prefers > > if (...) > direct_func_1() > else > direct_func_2() > > to > > tp->func() > > For hot paths, branch prediction will almost always predict > the correct > path, without any need for deferenced, indirect jumps. > > The latter example may look more clean, but the former is probably > faster in Real Life(tm). > Not measured. But with so many different workaround methods (TG3_FLAG_MBOX_WRITE_REORDER, TG3_FLAG_TXD_MBOX_HWBUG, TG3_FLG2_ICH_WORKAROUND, TG3_FLAG_5701_REG_WRITE_BUG, etc), it's more like: if (...) direct_func_1() else if (...) direct_func_2() else if (...) direct_func_3() else direct_func_4() At some point I suspect the indirect function pointer method will become better. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
From: Jeff Garzik <[EMAIL PROTECTED]> Date: Wed, 27 Jul 2005 23:44:51 -0400 > In x86-based CPUs at least (the largest tg3 platform), branch prediction > often prefers > > if (...) > direct_func_1() > else > direct_func_2() > > to > > tp->func() Indirect function calls also kill cpus such as ia64, which cannot avoid the implicit branch prediction miss unless the function method target is prefetched several instruction groups before the call and gcc does not emit the necessary directives to achieve this even if it were possible. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6 1/5] tg3: Add basic register access function pointers
Michael Chan wrote: This patch adds the basic function pointers to do register accesses in the fast path. This was suggested by David Miller. The idea is that various register access methods for different hardware errata can easily be implemented with these function pointers and performance will not be degraded on chips that use normal register access methods. Is this theory, or it has been actually measured? In x86-based CPUs at least (the largest tg3 platform), branch prediction often prefers if (...) direct_func_1() else direct_func_2() to tp->func() For hot paths, branch prediction will almost always predict the correct path, without any need for deferenced, indirect jumps. The latter example may look more clean, but the former is probably faster in Real Life(tm). Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6 1/5] tg3: Add basic register access function pointers
This patch adds the basic function pointers to do register accesses in
the fast path. This was suggested by David Miller. The idea is that
various register access methods for different hardware errata can easily
be implemented with these function pointers and performance will not be
degraded on chips that use normal register access methods.
The various register read write macros (e.g. tw32, tr32, tw32_mailbox)
are redefined to call the function pointers.
Signed-off-by: Michael Chan <[EMAIL PROTECTED]>
diff -Nrup 1/drivers/net/tg3.c 2/drivers/net/tg3.c
--- 1/drivers/net/tg3.c 2005-07-25 20:01:38.0 -0700
+++ 2/drivers/net/tg3.c 2005-07-26 07:33:32.0 -0700
@@ -366,7 +366,7 @@ static void _tw32_flush(struct tg3 *tp,
}
}
-static inline void _tw32_rx_mbox(struct tg3 *tp, u32 off, u32 val)
+static void tg3_write32_rx_mbox(struct tg3 *tp, u32 off, u32 val)
{
void __iomem *mbox = tp->regs + off;
writel(val, mbox);
@@ -374,7 +374,7 @@ static inline void _tw32_rx_mbox(struct
readl(mbox);
}
-static inline void _tw32_tx_mbox(struct tg3 *tp, u32 off, u32 val)
+static void tg3_write32_tx_mbox(struct tg3 *tp, u32 off, u32 val)
{
void __iomem *mbox = tp->regs + off;
writel(val, mbox);
@@ -384,17 +384,23 @@ static inline void _tw32_tx_mbox(struct
readl(mbox);
}
-#define tw32_mailbox(reg, val) writel(((val) & 0x), tp->regs + (reg))
-#define tw32_rx_mbox(reg, val) _tw32_rx_mbox(tp, reg, val)
-#define tw32_tx_mbox(reg, val) _tw32_tx_mbox(tp, reg, val)
+static void tg3_write32(struct tg3 *tp, u32 off, u32 val)
+{
+ writel(val, tp->regs + off);
+}
-#define tw32(reg,val) tg3_write_indirect_reg32(tp,(reg),(val))
+static u32 tg3_read32(struct tg3 *tp, u32 off)
+{
+ return (readl(tp->regs + off));
+}
+
+#define tw32_mailbox(reg, val) tp->write32_mbox(tp, reg, val)
+#define tw32_rx_mbox(reg, val) tp->write32_rx_mbox(tp, reg, val)
+#define tw32_tx_mbox(reg, val) tp->write32_tx_mbox(tp, reg, val)
+
+#define tw32(reg,val) tp->write32(tp, reg, val)
#define tw32_f(reg,val)_tw32_flush(tp,(reg),(val))
-#define tw16(reg,val) writew(((val) & 0x), tp->regs + (reg))
-#define tw8(reg,val) writeb(((val) & 0xff), tp->regs + (reg))
-#define tr32(reg) readl(tp->regs + (reg))
-#define tr16(reg) readw(tp->regs + (reg))
-#define tr8(reg) readb(tp->regs + (reg))
+#define tr32(reg) tp->read32(tp, reg)
static void tg3_write_mem(struct tg3 *tp, u32 off, u32 val)
{
@@ -9325,6 +9331,12 @@ static int __devinit tg3_get_invariants(
pci_write_config_dword(tp->pdev, TG3PCI_PCISTATE,
pci_state_reg);
}
+ tp->read32 = tg3_read32;
+ tp->write32 = tg3_write_indirect_reg32;
+ tp->write32_mbox = tg3_write32;
+ tp->write32_tx_mbox = tg3_write32_tx_mbox;
+ tp->write32_rx_mbox = tg3_write32_rx_mbox;
+
/* Get eeprom hw config before calling tg3_set_power_state().
* In particular, the TG3_FLAG_EEPROM_WRITE_PROT flag must be
* determined before calling tg3_set_power_state() so that
diff -Nrup 1/drivers/net/tg3.h 2/drivers/net/tg3.h
--- 1/drivers/net/tg3.h 2005-07-25 20:01:38.0 -0700
+++ 2/drivers/net/tg3.h 2005-07-25 20:05:53.0 -0700
@@ -2049,6 +2049,10 @@ struct tg3 {
spinlock_t lock;
spinlock_t indirect_lock;
+ u32 (*read32) (struct tg3 *, u32);
+ void(*write32) (struct tg3 *, u32, u32);
+ void(*write32_mbox) (struct tg3 *, u32,
+u32);
void __iomem*regs;
struct net_device *dev;
struct pci_dev *pdev;
@@ -2060,6 +2064,8 @@ struct tg3 {
u32 msg_enable;
/* begin "tx thread" cacheline section */
+ void(*write32_tx_mbox) (struct tg3 *, u32,
+ u32);
u32 tx_prod;
u32 tx_cons;
u32 tx_pending;
@@ -2071,6 +2077,8 @@ struct tg3 {
dma_addr_t tx_desc_mapping;
/* begin "rx thread" cacheline section */
+ void(*write32_rx_mbox) (struct tg3 *, u32,
+ u32);
u32 rx_rcb_ptr;
u32 rx_std_ptr;
u32 rx_jumbo_ptr;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
