[B.A.T.M.A.N.] still jiffies - msecs mixup?

[B.A.T.M.A.N.] still jiffies - msecs mixup?

[Linus Lüssing]

Hmm, while scanning through the code, there were some things that
looked a little odd to me...
---
chedule_own_packet() (/ schedule_forward_packet() )
  -> send_time = own_global_send_time(bat_priv) (/forward_send_time(bat_priv))
     <- returns current jiffies + orig_interval(in ms!) from bat_priv + jitter(in ms?)

  -> can_aggregate_with(send_time)
     -> converts MAX_AGGREGATION_MS to jiffies, seems to expect send_time to be in jiffies format

  -> new_aggregated_packet(send_time)
     -> queue_delayed_work(send_time - jiffies), 
        -> so, queue_delayed_work(orig_interval + jitter - time_diff_in_jiffies + MAX_AGGREGATION(in jiffies))?
---

Looks like we'd have some portions in the send_time still in
jiffies format, though it should be a time purely in msecs for
queue_delayed_work if I'm not mistaken.
If this is really a bug, would it be better to deal with jiffies
in these places (so converting orig_interval to jiffies) and
convert it back when queueing the work, or better convert the
jiffies variable to msecs and skipping all the other
msecs_to_jiffies conversions?

Cheers, Linus

Re: [B.A.T.M.A.N.] still jiffies - msecs mixup?

[Marek Lindner]

On Saturday 19 June 2010 17:15:40 Linus Lüssing wrote:
> schedule_own_packet() (/ schedule_forward_packet() )
>   -> send_time = own_global_send_time(bat_priv)
> (/forward_send_time(bat_priv)) <- returns current jiffies +
> orig_interval(in ms!) from bat_priv + jitter(in ms?)

The orig_interval and jitter are transformed to jiffies (towards the end of the 
line). It is not very obvious and might be susceptible for overflows. We 
probably should use msecs_to_jiffies() here.


>   -> can_aggregate_with(send_time)
>      -> converts MAX_AGGREGATION_MS to jiffies, seems to expect send_time
> to be in jiffies format

Correct - we have jiffies here.


>   -> new_aggregated_packet(send_time)
>      -> queue_delayed_work(send_time - jiffies),
>         -> so, queue_delayed_work(orig_interval + jitter -
> time_diff_in_jiffies + MAX_AGGREGATION(in jiffies))? ---
> 
> Looks like we'd have some portions in the send_time still in
> jiffies format, though it should be a time purely in msecs for
> queue_delayed_work if I'm not mistaken.

You are mistaken.  :-)
workqueue.c:323 [right above queue_delayed_work()] says: 
@delay: number of jiffies to wait before queueing

Regards,
Marek