RE: [PATCH V3] suspend: enable freeze timeout configuration throughsys

[Liu, Chuansheng]

> >>>>>> -#define TIMEOUT    (20 * HZ)
> >>>>>> +unsigned int __read_mostly sys_freeze_process_timeout_msecs =
> >> 20000;
> >>>>>
> >>>>> 20000 does not mean 20 seconds since we can select HZ other than
> 1000.
> >>>>> So (20 * HZ) is better than 20000.
> >>>>>
> >>>> [Li, Fei]
> >>>> Are you sure about this, (20*HZ) better than 20000, or you mean 20 *
> >> MSEC_PER_SEC?
> >>> Yasuaki  mean HZ value will not always be 1000.The value of HZ differs for
> >> each
> >>>   supported architecture. In fact, on some supported architectures,
> >>> it even differs between machine types.
> >>> When writing kernel code, never assume that HZ has any given value.
> >>> Right now you are assuming that the delay will be always 20 seconds
> because
> >> of
> >>> your assumption of HZ.
> >>
> >> That's correct, the initial value should be 20 * HZ (i.e. as before).
> > [Li, Fei]
> > Yes, you are right, and IMHO it's already as this in the patch,
> > as 20 * HZ == msecs_to_jiffies(20000), with the current definition
> MSEC_PER_SEC
> > of 1000L. I'll update the default value as 20 * MSEC_PER_SEC in patch V4.
> 
> 20 * MSEC_PER_SEC is not 20 seconds. In Linux, 1 * HZ is 1 seconds.
Sure.

> Thus,
>   - If HZ is defined as 1000, 1000 is 1 seconds.
>   - If HZ is defined as 250, 250 is 1 seconds.
> 
> 20 * MSEC_PER_SEC is always 20000.

> Thus,
>   - If HZ is defined as 1000, 20 * MSEC_PER_SEC is 20 seconds.
>   - If HZ is defined as 250, 20 * MSEC_PER_SEC is 80 seconds.
Could you check the patch again?
The patch are using msecs_to_jiffies(20 * MSEC_PER_SEC), not 20 * MSEC_PER_SEC.
msecs_to_jiffies() has done the conversion from MSEC_PER_SEC to HZ.

> 
> So you should use 20 * HZ if you define timeout at 20 seconds.
> 
> Thanks,
> Yasuaki Ishimatsu.

èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—