still jiffies - msecs mixup?
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
Quoted 4 lines · 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!)frombat_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.
Quoted 3 lines
->
can_aggregate_with(send_time)-> converts MAX_AGGREGATION_MS to jiffies, seems to expect send_timeto be in jiffies format
Correct - we have jiffies here.
Quoted 8 lines
->
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_timestill in jiffies format, though it should be a time purely in msecs forqueue_delayed_workif 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