| Message ID | 1456393950-4129-1-git-send-email-hongbo.zhang@linaro.org |
|---|---|
| State | New |
| Headers | show |
Please notice that this patch can only solve segfault to let odp run. If there is no speed in model string, system validation will report cpu max hz fail in such a case. Full propose is to use cpufreq under /sys/devices/system/cpu/, advantage of it is that this can be a common way for all platforms, but this isn't always enabled on some platforms and virtual machines. Then if cpufreq isn't there, we can fall back to each platform specific cpuinfo parse, if there isn't any max freq info in model string, the 'cpu mhz' is max freq now because when cpufreq isn't enabled, cpus always run at max speed. If we still plan to keep cpu hz api, it deserves to do such a modification, but now I was using a ubuntu running in vm, don't have cpufreq sysfs to test with, and what's more, I won't have time to spend on this... On 25 February 2016 at 17:52, <hongbo.zhang@linaro.org> wrote: > From: Hongbo Zhang <hongbo.zhang@linaro.org> > > This is for https://bugs.linaro.org/show_bug.cgi?id=2033 > If the model string doesn't include speed info, segfault should > be avoided. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org> > --- > platform/linux-generic/arch/x86/odp_sysinfo_parse.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > index 2ef49e4..c1e05c0 100644 > --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > @@ -24,10 +24,12 @@ int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo) > sizeof(sysinfo->model_str[id]) - 1); > > pos = strchr(sysinfo->model_str[id], '@'); > - *(pos - 1) = '\0'; > - if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { > - hz = (uint64_t)(ghz * 1000000000.0); > - sysinfo->cpu_hz_max[id] = hz; > + if (pos) { > + *(pos - 1) = '\0'; > + if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { > + hz = (uint64_t)(ghz * 1000000000.0); > + sysinfo->cpu_hz_max[id] = hz; > + } > } > id++; > } > -- > 2.1.4 >
please review. Maxim. On 02/25/16 12:52, hongbo.zhang@linaro.org wrote: > From: Hongbo Zhang <hongbo.zhang@linaro.org> > > This is for https://bugs.linaro.org/show_bug.cgi?id=2033 > If the model string doesn't include speed info, segfault should > be avoided. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org> > --- > platform/linux-generic/arch/x86/odp_sysinfo_parse.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > index 2ef49e4..c1e05c0 100644 > --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c > @@ -24,10 +24,12 @@ int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo) > sizeof(sysinfo->model_str[id]) - 1); > > pos = strchr(sysinfo->model_str[id], '@'); > - *(pos - 1) = '\0'; > - if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { > - hz = (uint64_t)(ghz * 1000000000.0); > - sysinfo->cpu_hz_max[id] = hz; > + if (pos) { > + *(pos - 1) = '\0'; > + if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { > + hz = (uint64_t)(ghz * 1000000000.0); > + sysinfo->cpu_hz_max[id] = hz; > + } > } > id++; > }
This patch seems stale and needs a rebase. I get this trying to apply it:
bill@Ubuntu15:~/linaro/hongbo$ git am --reject ~/Mail/Incoming/Hongbo/1
Applying: linux-generic: make x86 cpuinfo parser more robust
Checking patch platform/linux-generic/arch/x86/odp_sysinfo_parse.c...
error: while searching for:
sizeof(sysinfo->model_str[id]) - 1);
pos = strchr(sysinfo->model_str[id], '@');
*(pos - 1) = '\0';
if (sscanf(pos, "@ %lfGHz", &ghz) == 1) {
hz = (uint64_t)(ghz * 1000000000.0);
sysinfo->cpu_hz_max[id] = hz;
}
id++;
}
error: patch failed: platform/linux-generic/arch/x86/odp_sysinfo_parse.c:24
Applying patch platform/linux-generic/arch/x86/odp_sysinfo_parse.c with 1
reject...
Rejected hunk #1.
Patch failed at 0001 linux-generic: make x86 cpuinfo parser more robust
The copy of the patch that failed is found in:
/home/bill/linaro/hongbo/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
On Thu, Mar 24, 2016 at 8:08 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:
> please review.
>
> Maxim.
>
>
> On 02/25/16 12:52, hongbo.zhang@linaro.org wrote:
>
>> From: Hongbo Zhang <hongbo.zhang@linaro.org>
>>
>> This is for https://bugs.linaro.org/show_bug.cgi?id=2033
>> If the model string doesn't include speed info, segfault should
>> be avoided.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>> platform/linux-generic/arch/x86/odp_sysinfo_parse.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> index 2ef49e4..c1e05c0 100644
>> --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> @@ -24,10 +24,12 @@ int odp_cpuinfo_parser(FILE *file, odp_system_info_t
>> *sysinfo)
>> sizeof(sysinfo->model_str[id]) - 1);
>> pos = strchr(sysinfo->model_str[id], '@');
>> - *(pos - 1) = '\0';
>> - if (sscanf(pos, "@ %lfGHz", &ghz) == 1) {
>> - hz = (uint64_t)(ghz * 1000000000.0);
>> - sysinfo->cpu_hz_max[id] = hz;
>> + if (pos) {
>> + *(pos - 1) = '\0';
>> + if (sscanf(pos, "@ %lfGHz", &ghz) == 1) {
>> + hz = (uint64_t)(ghz *
>> 1000000000.0);
>> + sysinfo->cpu_hz_max[id] = hz;
>> + }
>> }
>> id++;
>> }
>>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c index 2ef49e4..c1e05c0 100644 --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c @@ -24,10 +24,12 @@ int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo) sizeof(sysinfo->model_str[id]) - 1); pos = strchr(sysinfo->model_str[id], '@'); - *(pos - 1) = '\0'; - if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { - hz = (uint64_t)(ghz * 1000000000.0); - sysinfo->cpu_hz_max[id] = hz; + if (pos) { + *(pos - 1) = '\0'; + if (sscanf(pos, "@ %lfGHz", &ghz) == 1) { + hz = (uint64_t)(ghz * 1000000000.0); + sysinfo->cpu_hz_max[id] = hz; + } } id++; }