linux-kernel - RE: [PATCH V3] suspend: enable freeze timeout configuration through sys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <27240C0AC20F114CBF8149A2696CBE4A2212E8@SHSMSX101.ccr.corp.intel.com>
Date:	Fri, 1 Feb 2013 02:02:21 +0000
From:	"Liu, Chuansheng" <chuansheng.liu@...el.com>
To:	Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>,
	"Li, Fei" <fei.li@...el.com>
CC:	"Rafael J. Wysocki" <rjw@...k.pl>,
	anish singh <anish198519851985@...il.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: RE: [PATCH V3] suspend: enable freeze timeout configuration through
 sys

> >>>>>> -#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.