* [patch 4/5] net: hip04: Make tx coalesce timer actually work [not found] <20150413210009.682000343@linutronix.de> @ 2015-04-13 21:02 ` Thomas Gleixner 2015-04-13 21:24 ` Arnd Bergmann 0 siblings, 1 reply; 11+ messages in thread From: Thomas Gleixner @ 2015-04-13 21:02 UTC (permalink / raw) To: LKML Cc: Peter Zijlstra, Ingo Molnar, David S. Miller, dingtianhong, Arnd Bergmann, Zhangfei Gao, Dan Carpenter, netdev [-- Attachment #1: net-hip04-fix-hrtimer-wreckage.patch --] [-- Type: text/plain, Size: 2869 bytes --] The code sets the expiry value of the timer to a relative value and starts it with hrtimer_start_expires. That's fine, but that only works once. The timer is started in relative mode, so the expiry value gets overwritten with the absolut expiry time (now + expiry). So once the timer expired, a new call to hrtimer_start_expires results in an immidiately expired timer, because the expiry value is already in the past. Use the proper mechanisms to (re)start the timer in the intended way. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: dingtianhong <dingtianhong@huawei.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Zhangfei Gao <zhangfei.gao@linaro.org> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: netdev@vger.kernel.org --- drivers/net/ethernet/hisilicon/hip04_eth.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c =================================================================== --- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c +++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c @@ -413,6 +413,15 @@ out: return count; } +static void hip04_start_tx_timer(struct hip04_priv *priv) +{ + ktime_t t; + + /* allow timer to fire after half the time at the earliest */ + t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); + hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL); +} + static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct hip04_priv *priv = netdev_priv(ndev); @@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s } } else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) { /* cleanup not pending yet, start a new timer */ - hrtimer_start_expires(&priv->tx_coalesce_timer, - HRTIMER_MODE_REL); + hip04_start_tx_timer(priv); } return NETDEV_TX_OK; @@ -549,7 +557,7 @@ done: /* clean up tx descriptors and start a new timer if necessary */ tx_remaining = hip04_tx_reclaim(ndev, false); if (rx < budget && tx_remaining) - hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL); + hip04_start_tx_timer(priv); return rx; } @@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo struct hip04_priv *priv; struct resource *res; unsigned int irq; - ktime_t txtime; int ret; ndev = alloc_etherdev(sizeof(struct hip04_priv)); @@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo */ priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4; priv->tx_coalesce_usecs = 200; - /* allow timer to fire after half the time at the earliest */ - txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); - hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime); priv->tx_coalesce_timer.function = tx_done; priv->map = syscon_node_to_regmap(arg.np); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 21:02 ` [patch 4/5] net: hip04: Make tx coalesce timer actually work Thomas Gleixner @ 2015-04-13 21:24 ` Arnd Bergmann 2015-04-13 21:42 ` Thomas Gleixner 0 siblings, 1 reply; 11+ messages in thread From: Arnd Bergmann @ 2015-04-13 21:24 UTC (permalink / raw) To: Thomas Gleixner Cc: LKML, Peter Zijlstra, Ingo Molnar, David S. Miller, dingtianhong, Zhangfei Gao, Dan Carpenter, netdev On Monday 13 April 2015 21:02:23 Thomas Gleixner wrote: > The code sets the expiry value of the timer to a relative value and > starts it with hrtimer_start_expires. That's fine, but that only works > once. The timer is started in relative mode, so the expiry value gets > overwritten with the absolut expiry time (now + expiry). > > So once the timer expired, a new call to hrtimer_start_expires results > in an immidiately expired timer, because the expiry value is > already in the past. > > Use the proper mechanisms to (re)start the timer in the intended way. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: dingtianhong <dingtianhong@huawei.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Zhangfei Gao <zhangfei.gao@linaro.org> > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Cc: netdev@vger.kernel.org Thanks a lot for the fix. The mistake was clearly mine, as I had sent a patch to introduce the tx coalesce timer without access to hardware or a way to test that what I did was correct. There are other known problems in the version of the driver that got merged, and I believe that someone is now looking at them. What I think we really want here is a way for user space to configure both the minimum and maximum coalesce timer separately rather than assuming half the time is what we want. Arnd > @@ -413,6 +413,15 @@ out: > return count; > } > > +static void hip04_start_tx_timer(struct hip04_priv *priv) > +{ > + ktime_t t; > + > + /* allow timer to fire after half the time at the earliest */ > + t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); > + hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL); > +} Question: this looks to me like it sets both the minimum and maximum time to priv->tx_coalesce_usecs/2, when the intention was to set the minimum to priv->tx_coalesce_usecs/2 and the maximum to priv->tx_coalesce_usecs. Am I missing something subtle here, or did you just misread my original intention from the botched code? Arnd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 21:24 ` Arnd Bergmann @ 2015-04-13 21:42 ` Thomas Gleixner 2015-04-13 22:03 ` Arnd Bergmann 0 siblings, 1 reply; 11+ messages in thread From: Thomas Gleixner @ 2015-04-13 21:42 UTC (permalink / raw) To: Arnd Bergmann Cc: LKML, Peter Zijlstra, Ingo Molnar, David S. Miller, dingtianhong, Zhangfei Gao, Dan Carpenter, netdev On Mon, 13 Apr 2015, Arnd Bergmann wrote: > On Monday 13 April 2015 21:02:23 Thomas Gleixner wrote: > > The code sets the expiry value of the timer to a relative value and > > starts it with hrtimer_start_expires. That's fine, but that only works > > once. The timer is started in relative mode, so the expiry value gets > > overwritten with the absolut expiry time (now + expiry). > > > > So once the timer expired, a new call to hrtimer_start_expires results > > in an immidiately expired timer, because the expiry value is > > already in the past. > > > > Use the proper mechanisms to (re)start the timer in the intended way. > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: dingtianhong <dingtianhong@huawei.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Zhangfei Gao <zhangfei.gao@linaro.org> > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > Cc: netdev@vger.kernel.org > > Thanks a lot for the fix. The mistake was clearly mine, as I had sent > a patch to introduce the tx coalesce timer without access to hardware > or a way to test that what I did was correct. > > There are other known problems in the version of the driver that got > merged, and I believe that someone is now looking at them. > > What I think we really want here is a way for user space to configure > both the minimum and maximum coalesce timer separately rather than > assuming half the time is what we want. > > Arnd > > > @@ -413,6 +413,15 @@ out: > > return count; > > } > > > > +static void hip04_start_tx_timer(struct hip04_priv *priv) > > +{ > > + ktime_t t; > > + > > + /* allow timer to fire after half the time at the earliest */ > > + t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); > > + hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL); > > +} > > Question: this looks to me like it sets both the minimum and maximum > time to priv->tx_coalesce_usecs/2, when the intention was to set > the minimum to priv->tx_coalesce_usecs/2 and the maximum to > priv->tx_coalesce_usecs. Am I missing something subtle here, or did > you just misread my original intention from the botched code? Yes, I missed that. Simple fix for this is: unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns), t_ns, HRTIMER_MODE_REL); Thanks, tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 21:42 ` Thomas Gleixner @ 2015-04-13 22:03 ` Arnd Bergmann 2015-04-13 22:08 ` Thomas Gleixner 0 siblings, 1 reply; 11+ messages in thread From: Arnd Bergmann @ 2015-04-13 22:03 UTC (permalink / raw) To: Thomas Gleixner Cc: LKML, Peter Zijlstra, Ingo Molnar, David S. Miller, dingtianhong, Zhangfei Gao, Dan Carpenter, netdev On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote: > > > > Question: this looks to me like it sets both the minimum and maximum > > time to priv->tx_coalesce_usecs/2, when the intention was to set > > the minimum to priv->tx_coalesce_usecs/2 and the maximum to > > priv->tx_coalesce_usecs. Am I missing something subtle here, or did > > you just misread my original intention from the botched code? > > Yes, I missed that. Simple fix for this is: > > unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; > > hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns), > t_ns, HRTIMER_MODE_REL); Ah, good. I have to admit that I'd probably make the same mistake again if I was to do this for another driver and you hadn't sent the fix. The hrtimer_set_expires_range() function just looked like it had been designed for the use case I was interested in ;-). Any idea how to prevent the next person from making the same mistake? Arnd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 22:03 ` Arnd Bergmann @ 2015-04-13 22:08 ` Thomas Gleixner 2015-04-14 7:53 ` Ding Tianhong 2015-04-14 18:15 ` David Miller 0 siblings, 2 replies; 11+ messages in thread From: Thomas Gleixner @ 2015-04-13 22:08 UTC (permalink / raw) To: Arnd Bergmann Cc: LKML, Peter Zijlstra, Ingo Molnar, David S. Miller, dingtianhong, Zhangfei Gao, Dan Carpenter, netdev On Tue, 14 Apr 2015, Arnd Bergmann wrote: > On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote: > > > > > > Question: this looks to me like it sets both the minimum and maximum > > > time to priv->tx_coalesce_usecs/2, when the intention was to set > > > the minimum to priv->tx_coalesce_usecs/2 and the maximum to > > > priv->tx_coalesce_usecs. Am I missing something subtle here, or did > > > you just misread my original intention from the botched code? > > > > Yes, I missed that. Simple fix for this is: > > > > unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; > > > > hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns), > > t_ns, HRTIMER_MODE_REL); > > Ah, good. I have to admit that I'd probably make the same mistake > again if I was to do this for another driver and you hadn't sent > the fix. The hrtimer_set_expires_range() function just looked like > it had been designed for the use case I was interested in ;-). > > Any idea how to prevent the next person from making the same mistake? Yes. Documentation :) Thanks, tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 22:08 ` Thomas Gleixner @ 2015-04-14 7:53 ` Ding Tianhong 2015-04-14 18:15 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: Ding Tianhong @ 2015-04-14 7:53 UTC (permalink / raw) To: Thomas Gleixner, Arnd Bergmann Cc: LKML, Peter Zijlstra, Ingo Molnar, David S. Miller, Zhangfei Gao, Dan Carpenter, netdev On 2015/4/14 6:08, Thomas Gleixner wrote: > On Tue, 14 Apr 2015, Arnd Bergmann wrote: >> On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote: >>>> >>>> Question: this looks to me like it sets both the minimum and maximum >>>> time to priv->tx_coalesce_usecs/2, when the intention was to set >>>> the minimum to priv->tx_coalesce_usecs/2 and the maximum to >>>> priv->tx_coalesce_usecs. Am I missing something subtle here, or did >>>> you just misread my original intention from the botched code? >>> >>> Yes, I missed that. Simple fix for this is: >>> >>> unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; >>> >>> hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns), >>> t_ns, HRTIMER_MODE_REL); >> >> Ah, good. I have to admit that I'd probably make the same mistake >> again if I was to do this for another driver and you hadn't sent >> the fix. The hrtimer_set_expires_range() function just looked like >> it had been designed for the use case I was interested in ;-). >> >> Any idea how to prevent the next person from making the same mistake? > > Yes. Documentation :) > Looks good to me, thanks everyone. Ding > Thanks, > > tglx > > . > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work 2015-04-13 22:08 ` Thomas Gleixner 2015-04-14 7:53 ` Ding Tianhong @ 2015-04-14 18:15 ` David Miller 2015-04-14 19:42 ` [patch v2] " Thomas Gleixner 1 sibling, 1 reply; 11+ messages in thread From: David Miller @ 2015-04-14 18:15 UTC (permalink / raw) To: tglx Cc: arnd, linux-kernel, peterz, mingo, dingtianhong, zhangfei.gao, dan.carpenter, netdev From: Thomas Gleixner <tglx@linutronix.de> Date: Tue, 14 Apr 2015 00:08:23 +0200 (CEST) > On Tue, 14 Apr 2015, Arnd Bergmann wrote: >> On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote: >> > > >> > > Question: this looks to me like it sets both the minimum and maximum >> > > time to priv->tx_coalesce_usecs/2, when the intention was to set >> > > the minimum to priv->tx_coalesce_usecs/2 and the maximum to >> > > priv->tx_coalesce_usecs. Am I missing something subtle here, or did >> > > you just misread my original intention from the botched code? >> > >> > Yes, I missed that. Simple fix for this is: >> > >> > unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; >> > >> > hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns), >> > t_ns, HRTIMER_MODE_REL); >> >> Ah, good. I have to admit that I'd probably make the same mistake >> again if I was to do this for another driver and you hadn't sent >> the fix. The hrtimer_set_expires_range() function just looked like >> it had been designed for the use case I was interested in ;-). >> >> Any idea how to prevent the next person from making the same mistake? > > Yes. Documentation :) Can I get a respin of this patch with the above? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch v2] net: hip04: Make tx coalesce timer actually work 2015-04-14 18:15 ` David Miller @ 2015-04-14 19:42 ` Thomas Gleixner 2015-04-15 2:24 ` Ding Tianhong ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Thomas Gleixner @ 2015-04-14 19:42 UTC (permalink / raw) To: David Miller Cc: arnd, linux-kernel, peterz, mingo, dingtianhong, zhangfei.gao, dan.carpenter, netdev The code sets the expiry value of the timer to a relative value and starts it with hrtimer_start_expires. That's fine, but that only works once. The timer is started in relative mode, so the expiry value gets overwritten with the absolut expiry time (now + expiry). So once the timer expired, a new call to hrtimer_start_expires results in an immidiately expired timer, because the expiry value is already in the past. Use the proper mechanisms to (re)start the timer in the intended way. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: dingtianhong <dingtianhong@huawei.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Zhangfei Gao <zhangfei.gao@linaro.org> Cc: Dan Carpenter <dan.carpenter@oracle.com> Cc: netdev@vger.kernel.org --- drivers/net/ethernet/hisilicon/hip04_eth.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c =================================================================== --- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c +++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c @@ -413,6 +413,15 @@ out: return count; } +static void hip04_start_tx_timer(struct hip04_priv *priv) +{ + unsigned long ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; + + /* allow timer to fire after half the time at the earliest */ + hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(ns), + ns, HRTIMER_MODE_REL); +} + static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct hip04_priv *priv = netdev_priv(ndev); @@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s } } else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) { /* cleanup not pending yet, start a new timer */ - hrtimer_start_expires(&priv->tx_coalesce_timer, - HRTIMER_MODE_REL); + hip04_start_tx_timer(priv); } return NETDEV_TX_OK; @@ -549,7 +557,7 @@ done: /* clean up tx descriptors and start a new timer if necessary */ tx_remaining = hip04_tx_reclaim(ndev, false); if (rx < budget && tx_remaining) - hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL); + hip04_start_tx_timer(priv); return rx; } @@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo struct hip04_priv *priv; struct resource *res; unsigned int irq; - ktime_t txtime; int ret; ndev = alloc_etherdev(sizeof(struct hip04_priv)); @@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo */ priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4; priv->tx_coalesce_usecs = 200; - /* allow timer to fire after half the time at the earliest */ - txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); - hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime); priv->tx_coalesce_timer.function = tx_done; priv->map = syscon_node_to_regmap(arg.np); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch v2] net: hip04: Make tx coalesce timer actually work 2015-04-14 19:42 ` [patch v2] " Thomas Gleixner @ 2015-04-15 2:24 ` Ding Tianhong 2015-04-15 10:20 ` Arnd Bergmann 2015-04-15 21:22 ` David Miller 2 siblings, 0 replies; 11+ messages in thread From: Ding Tianhong @ 2015-04-15 2:24 UTC (permalink / raw) To: Thomas Gleixner, David Miller Cc: arnd, linux-kernel, peterz, mingo, zhangfei.gao, dan.carpenter, netdev On 2015/4/15 3:42, Thomas Gleixner wrote: > The code sets the expiry value of the timer to a relative value and > starts it with hrtimer_start_expires. That's fine, but that only works > once. The timer is started in relative mode, so the expiry value gets > overwritten with the absolut expiry time (now + expiry). > > So once the timer expired, a new call to hrtimer_start_expires results > in an immidiately expired timer, because the expiry value is > already in the past. > > Use the proper mechanisms to (re)start the timer in the intended way. > Acked-by: Ding Tianhong <dingtianhong@huawei.com> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: dingtianhong <dingtianhong@huawei.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Zhangfei Gao <zhangfei.gao@linaro.org> > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Cc: netdev@vger.kernel.org > --- > drivers/net/ethernet/hisilicon/hip04_eth.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c > =================================================================== > --- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c > +++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c > @@ -413,6 +413,15 @@ out: > return count; > } > > +static void hip04_start_tx_timer(struct hip04_priv *priv) > +{ > + unsigned long ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2; > + > + /* allow timer to fire after half the time at the earliest */ > + hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(ns), > + ns, HRTIMER_MODE_REL); > +} > + > static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev) > { > struct hip04_priv *priv = netdev_priv(ndev); > @@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s > } > } else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) { > /* cleanup not pending yet, start a new timer */ > - hrtimer_start_expires(&priv->tx_coalesce_timer, > - HRTIMER_MODE_REL); > + hip04_start_tx_timer(priv); > } > > return NETDEV_TX_OK; > @@ -549,7 +557,7 @@ done: > /* clean up tx descriptors and start a new timer if necessary */ > tx_remaining = hip04_tx_reclaim(ndev, false); > if (rx < budget && tx_remaining) > - hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL); > + hip04_start_tx_timer(priv); > > return rx; > } > @@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo > struct hip04_priv *priv; > struct resource *res; > unsigned int irq; > - ktime_t txtime; > int ret; > > ndev = alloc_etherdev(sizeof(struct hip04_priv)); > @@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo > */ > priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4; > priv->tx_coalesce_usecs = 200; > - /* allow timer to fire after half the time at the earliest */ > - txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2); > - hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime); > priv->tx_coalesce_timer.function = tx_done; > > priv->map = syscon_node_to_regmap(arg.np); > > . > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch v2] net: hip04: Make tx coalesce timer actually work 2015-04-14 19:42 ` [patch v2] " Thomas Gleixner 2015-04-15 2:24 ` Ding Tianhong @ 2015-04-15 10:20 ` Arnd Bergmann 2015-04-15 21:22 ` David Miller 2 siblings, 0 replies; 11+ messages in thread From: Arnd Bergmann @ 2015-04-15 10:20 UTC (permalink / raw) To: Thomas Gleixner Cc: David Miller, linux-kernel, peterz, mingo, dingtianhong, zhangfei.gao, dan.carpenter, netdev On Tuesday 14 April 2015 21:42:42 Thomas Gleixner wrote: > The code sets the expiry value of the timer to a relative value and > starts it with hrtimer_start_expires. That's fine, but that only works > once. The timer is started in relative mode, so the expiry value gets > overwritten with the absolut expiry time (now + expiry). > > So once the timer expired, a new call to hrtimer_start_expires results > in an immidiately expired timer, because the expiry value is > already in the past. > > Use the proper mechanisms to (re)start the timer in the intended way. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: dingtianhong <dingtianhong@huawei.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Zhangfei Gao <zhangfei.gao@linaro.org> > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Cc: netdev@vger.kernel.org > Acked-by: Arnd Bergmann <arnd@arndb.de> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch v2] net: hip04: Make tx coalesce timer actually work 2015-04-14 19:42 ` [patch v2] " Thomas Gleixner 2015-04-15 2:24 ` Ding Tianhong 2015-04-15 10:20 ` Arnd Bergmann @ 2015-04-15 21:22 ` David Miller 2 siblings, 0 replies; 11+ messages in thread From: David Miller @ 2015-04-15 21:22 UTC (permalink / raw) To: tglx Cc: arnd, linux-kernel, peterz, mingo, dingtianhong, zhangfei.gao, dan.carpenter, netdev From: Thomas Gleixner <tglx@linutronix.de> Date: Tue, 14 Apr 2015 21:42:42 +0200 (CEST) > The code sets the expiry value of the timer to a relative value and > starts it with hrtimer_start_expires. That's fine, but that only works > once. The timer is started in relative mode, so the expiry value gets > overwritten with the absolut expiry time (now + expiry). > > So once the timer expired, a new call to hrtimer_start_expires results > in an immidiately expired timer, because the expiry value is > already in the past. > > Use the proper mechanisms to (re)start the timer in the intended way. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Applied, thanks Thomas. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-04-15 21:22 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20150413210009.682000343@linutronix.de>
2015-04-13 21:02 ` [patch 4/5] net: hip04: Make tx coalesce timer actually work Thomas Gleixner
2015-04-13 21:24 ` Arnd Bergmann
2015-04-13 21:42 ` Thomas Gleixner
2015-04-13 22:03 ` Arnd Bergmann
2015-04-13 22:08 ` Thomas Gleixner
2015-04-14 7:53 ` Ding Tianhong
2015-04-14 18:15 ` David Miller
2015-04-14 19:42 ` [patch v2] " Thomas Gleixner
2015-04-15 2:24 ` Ding Tianhong
2015-04-15 10:20 ` Arnd Bergmann
2015-04-15 21:22 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).