[KJ] Timer code duplication in driver

[KJ] Timer code duplication in driver

[Oliver Korpilla]

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]

Hello!

I was just reviewing code about high resolution timers for learning to 
use their API correctly, and have come to the following code duplication 
(in 2.6.12.4 kernel.org):

drivers/char/ipmi/ipmi_si_intf.c:

static inline void add_usec_to_timer(struct timer_list *t, long v)
{
    t->sub_expires += nsec_to_arch_cycle(v * 1000);
    while (t->sub_expires >= arch_cycles_per_jiffy)
    {
        t->expires++;
        t->sub_expires -= arch_cycles_per_jiffy;
    }
}

And the standard methods to do this are defined as:

include/linux/posix-timers.h:

static inline void
normalize_jiffies(unsigned long *jiff, long *sub_jif)
{
    while ((*(sub_jif) - arch_cycles_per_jiffy) >= 0) {
        *(sub_jif) -= arch_cycles_per_jiffy;
        (*(jiff))++;
    }
}
static inline void
full_normalize_jiffies(unsigned long *jiff, long *sub_jif)
{
    normalize_jiffies(jiff, sub_jif);
    while (*(sub_jif) < 0) {
        *(sub_jif) += arch_cycles_per_jiffy;
        (*(jiff))--;
    }
}

So wouldn't a better version be:

static inline void add_usec_to_timer(struct timer_list *t, long v)
{
    t->sub_expires += nsec_to_arch_cycle(v * 1000);
    full_normalize_jiffies(&(t->expires), &(t->sub_expires));
}

Anyone else thinking this should be fixed?

Could someone please review this?

Thanks and with kind regards,
Oliver Korpilla

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Timer code duplication in driver

[Nishanth Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On 16.08.2005 [14:53:43 +0200], Oliver Korpilla wrote:
> Hello!
> 
> I was just reviewing code about high resolution timers for learning to 
> use their API correctly, and have come to the following code duplication 
> (in 2.6.12.4 kernel.org):

<snip>

> Anyone else thinking this should be fixed?
> 
> Could someone please review this?

Your best bet is to write a patch and send it to Corey Minyard, George
Anzinger and the high-res-timers list, cc-ing KJ again... I think you
are right, but it's Corey's code and George's subsystem.

Thanks,
Nish

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors