Vasu Dev wrote:
> On Wed, 2010-05-19 at 11:16 -0700, Joe Eykholt wrote:
>>>> */
>>>> -static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
>>>> +static unsigned long fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
>>>> {
>>>> struct fcoe_fcf *fcf;
>>>> struct fcoe_fcf *next;
>>>> + unsigned long next_timer = jiffies +
>> msecs_to_jiffies(FIP_DEF_FKA);
>>
>> Also, this should be further out than 8 seconds, in case there's
>> nothing sooner needed. That's why I used the 90 second time before.
>> The rest of the code in age_fcfs or the timeout should come up with
>> a closer value unless the 90 second keep-alive is the next thing
>> needed
>> or there's nothing at all to do.
>>
>
> This default value mostly gets written when fd_flag is zero due to
> sooner E node KA with typical 8 sec KA period and large value was always
> effective in case fd_flag set that was an issue since in that case
> periodic ka timeouts didn't kick after 90 second of fcf selection while
> fd_flag set.
I don't really understand. If there's any FCF at all, it should set
next_timer to the 1.5 * FKA since the oldest advertisment received.
With a selected FCF that has the flag on it should fire
every 1.5 * FKA to check for a missing advertisement.
With the change above, it'll fire after 1 FKA, find nothing todo
and then reschedule for .5 FKA later, right?
I may have missed something.
> I can roll back this change but then have to set KA period timeout for
> this case when fcf is selected in fcoe_ctlr_timeout, so it would add few
> more lines there after fcoe_ctlr_select calling as:-
>
> if (sel && sel->fd_flags &&
> time_after(next_timer, sel->time + sel->fka_period))
> next_timer = sel->time + fcf->fka_period;
But the code you added in age_fcfs also does something like that:
if (fcf->fd_flags &&
time_after(next_timer, jiffies + fcf->fka_period))
next_timer = jiffies + fcf->fka_period;
This is after checks that the FCF is selected. It's setting the time
to now + FKA, not the last advertisement + FKA, though.
I'm not sure why that's needed, either. Since other code already accounts
for the 1.5 * FKA check and the 2.5 * FKA timeout.
> I'm fine with either and let me know if you want me to change this.
I would put back the change to initializing next_timer, and take out
the code quoted above. Is that OK? Again, I may have misunderstood something.
Thanks,
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
