| Message ID | 20200831233558.71417-3-snelson@pensando.io |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | ionic: struct cleanups | expand |
On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote: > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h > index 9e2ac2b8a082..2b2eb5f2a0e5 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h > +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h > @@ -16,7 +16,7 @@ > #define IONIC_DEF_TXRX_DESC 4096 > #define IONIC_LIFS_MAX 1024 > #define IONIC_WATCHDOG_SECS 5 > -#define IONIC_ITR_COAL_USEC_DEFAULT 64 > +#define IONIC_ITR_COAL_USEC_DEFAULT 8 8 us interrupt coalescing does not hurt general operations?! No way. It's your customers who'll get hurt here, so your call, but I seriously doubt this. Unless the unit is not usec?
From: Jakub Kicinski <kuba@kernel.org> Date: Mon, 31 Aug 2020 16:50:54 -0700 > On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote: >> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h >> index 9e2ac2b8a082..2b2eb5f2a0e5 100644 >> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h >> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h >> @@ -16,7 +16,7 @@ >> #define IONIC_DEF_TXRX_DESC 4096 >> #define IONIC_LIFS_MAX 1024 >> #define IONIC_WATCHDOG_SECS 5 >> -#define IONIC_ITR_COAL_USEC_DEFAULT 64 >> +#define IONIC_ITR_COAL_USEC_DEFAULT 8 > > 8 us interrupt coalescing does not hurt general operations?! No way. > > It's your customers who'll get hurt here, so your call, but I seriously > doubt this. Unless the unit is not usec? Agreed, 8usec is really really low. You won't get much coalescing during bulk transfers with a value like that, eliminating the gain from coalescing in the first place.
On 8/31/20 5:16 PM, David Miller wrote: > From: Jakub Kicinski <kuba@kernel.org> > Date: Mon, 31 Aug 2020 16:50:54 -0700 > >> On Mon, 31 Aug 2020 16:35:55 -0700 Shannon Nelson wrote: >>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h >>> index 9e2ac2b8a082..2b2eb5f2a0e5 100644 >>> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h >>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h >>> @@ -16,7 +16,7 @@ >>> #define IONIC_DEF_TXRX_DESC 4096 >>> #define IONIC_LIFS_MAX 1024 >>> #define IONIC_WATCHDOG_SECS 5 >>> -#define IONIC_ITR_COAL_USEC_DEFAULT 64 >>> +#define IONIC_ITR_COAL_USEC_DEFAULT 8 >> 8 us interrupt coalescing does not hurt general operations?! No way. >> >> It's your customers who'll get hurt here, so your call, but I seriously >> doubt this. Unless the unit is not usec? > Agreed, 8usec is really really low. You won't get much coalescing during > bulk transfers with a value like that, eliminating the gain from coalescing > in the first place. Thanks. I'll drop this patch and come back to this issue when we get a chance to add adaptive coalescing. sln
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h index 9e2ac2b8a082..2b2eb5f2a0e5 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h @@ -16,7 +16,7 @@ #define IONIC_DEF_TXRX_DESC 4096 #define IONIC_LIFS_MAX 1024 #define IONIC_WATCHDOG_SECS 5 -#define IONIC_ITR_COAL_USEC_DEFAULT 64 +#define IONIC_ITR_COAL_USEC_DEFAULT 8 #define IONIC_DEV_CMD_REG_VERSION 1 #define IONIC_DEV_INFO_REG_COUNT 32
We've found that a smaller default value for interrupt coalescing works better for latency without hurting general operations. Signed-off-by: Shannon Nelson <snelson@pensando.io> --- drivers/net/ethernet/pensando/ionic/ionic_dev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)