| Message ID | 1400789466-11156-1-git-send-email-manuel.schoelling@gmx.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Dan Williams |
| Headers | show |
On Thu, May 22, 2014 at 1:11 PM, Manuel Schölling <manuel.schoelling@gmx.de> wrote: > To be future-proof and for better readability the time comparisons are modified > to use time_before() instead of plain, error-prone math. > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > --- > drivers/dma/ioat/dma_v2.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c > index 8d10580..fdd60c7 100644 > --- a/drivers/dma/ioat/dma_v2.c > +++ b/drivers/dma/ioat/dma_v2.c > @@ -735,7 +735,8 @@ int ioat2_check_space_lock(struct ioat2_dma_chan *ioat, int num_descs) > * called under bh_disabled so we need to trigger the timer > * event directly > */ > - if (jiffies > chan->timer.expires && timer_pending(&chan->timer)) { > + if (time_before(chan->timer.expires, jiffies) > + && timer_pending(&chan->timer)) { > struct ioatdma_device *device = chan->device; Thanks, let's use time_is_before_jiffies() for this cleanup... I'll fix up and apply. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2014-08-21 at 10:10 -0700, Dan Williams wrote: > On Thu, May 22, 2014 at 1:11 PM, Manuel Schölling > <manuel.schoelling@gmx.de> wrote: > > To be future-proof and for better readability the time comparisons are modified > > to use time_before() instead of plain, error-prone math. [] > > diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c [] > > @@ -735,7 +735,8 @@ int ioat2_check_space_lock(struct ioat2_dma_chan *ioat, int num_descs) > > * called under bh_disabled so we need to trigger the timer > > * event directly > > */ > > - if (jiffies > chan->timer.expires && timer_pending(&chan->timer)) { > > + if (time_before(chan->timer.expires, jiffies) > > + && timer_pending(&chan->timer)) { > > struct ioatdma_device *device = chan->device; > > Thanks, let's use time_is_before_jiffies() for this cleanup... I'll > fix up and apply. There are thousands of uses of time_before( and time_after( with jiffies, and 6 years after being added, a little more than a dozen or so of time_is_[before|after]_jiffies. I also think the "time_is_[before|after]_jiffies" macros are not very well named. Using them is more text than using jiffies directly. There is a small benefit in argument ordering correctness. time_after(jiffies, foo) time_is_after_jiffies(foo) I'd rather just drop jiffies altogether time_is_after(foo) or maybe add and use jiffies_is_after(foo) (all the other after_eq/before/before_eq variants too). -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 21 Aug 2014, Joe Perches wrote: > On Thu, 2014-08-21 at 10:10 -0700, Dan Williams wrote: > > On Thu, May 22, 2014 at 1:11 PM, Manuel Schölling > > <manuel.schoelling@gmx.de> wrote: > > > To be future-proof and for better readability the time comparisons are modified > > > to use time_before() instead of plain, error-prone math. > [] > > > diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c > [] > > > @@ -735,7 +735,8 @@ int ioat2_check_space_lock(struct ioat2_dma_chan *ioat, int num_descs) > > > * called under bh_disabled so we need to trigger the timer > > > * event directly > > > */ > > > - if (jiffies > chan->timer.expires && timer_pending(&chan->timer)) { > > > + if (time_before(chan->timer.expires, jiffies) > > > + && timer_pending(&chan->timer)) { > > > struct ioatdma_device *device = chan->device; > > > > Thanks, let's use time_is_before_jiffies() for this cleanup... I'll > > fix up and apply. > > There are thousands of uses of time_before( and time_after( > with jiffies, and 6 years after being added, a little more > than a dozen or so of time_is_[before|after]_jiffies. > > I also think the "time_is_[before|after]_jiffies" macros are > not very well named. > > Using them is more text than using jiffies directly. > There is a small benefit in argument ordering correctness. > > time_after(jiffies, foo) > time_is_after_jiffies(foo) > > I'd rather just drop jiffies altogether > time_is_after(foo) > or maybe add and use > jiffies_is_after(foo) jiffies_is_after is very understandable. The others seem to require some thought. julia
diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c index 8d10580..fdd60c7 100644 --- a/drivers/dma/ioat/dma_v2.c +++ b/drivers/dma/ioat/dma_v2.c @@ -735,7 +735,8 @@ int ioat2_check_space_lock(struct ioat2_dma_chan *ioat, int num_descs) * called under bh_disabled so we need to trigger the timer * event directly */ - if (jiffies > chan->timer.expires && timer_pending(&chan->timer)) { + if (time_before(chan->timer.expires, jiffies) + && timer_pending(&chan->timer)) { struct ioatdma_device *device = chan->device; mod_timer(&chan->timer, jiffies + COMPLETION_TIMEOUT);
To be future-proof and for better readability the time comparisons are modified to use time_before() instead of plain, error-prone math. Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> --- drivers/dma/ioat/dma_v2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)