On Tue, 3 May 2011 10:55:40 +0200
Robert Richter <robert.richter@xxxxxxx> wrote:
> On 02.05.11 09:48:05, Nicolas Kaiser wrote:
> > Checking 'oprofile_min_interval < 0' and
> > 'oprofile_max_interval < 0' doesn't work because
> > 'oprofile_min_interval' and 'oprofile_max_interval' are unsigned.
>
> max/min_interval are through all the code always unsigned. I don't
> know how min/max_sampl_rate in struct hws_qsi_info_block is spec'ed,
> but there it is unsigned too.
>
> So the best would be to return qsi.min/max_sampl_rate in
> hwsampler_query_min/max_interval() directly with no error codes as
> unsigned longs and to change the code in oprofile_hwsampler_init() to
> check for null. Both functions hwsampler_query_min/max_interval()
> could be moved to hwsampler.h as static inline functions. This makes
> the code also easier.
>
> This patch does not handle the null value case and the data truncation
> by casting from unsigned to singed is not fixed.
Ok, the improved patch now looks like this:
--
Subject: [PATCH] s390: oprofile: fix min/max interval query checks
From: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
oprofile_min_interval and oprofile_max_interval are unsigned, checking
for negative values doesn't work. Change hwsampler_query_min_interval
and hwsampler_query_max_interval to return an unsigned long and
check for a zero value instead.
Reported-by: Nicolas Kaiser <nikai@xxxxxxxxx>
Signed-off-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
---
arch/s390/oprofile/hwsampler.c | 14 ++++----------
arch/s390/oprofile/hwsampler.h | 4 ++--
arch/s390/oprofile/init.c | 8 ++------
3 files changed, 8 insertions(+), 18 deletions(-)
diff -urpN linux-2.6/arch/s390/oprofile/hwsampler.c linux-2.6-patched/arch/s390/oprofile/hwsampler.c
--- linux-2.6/arch/s390/oprofile/hwsampler.c 2011-05-10 10:53:04.000000000 +0200
+++ linux-2.6-patched/arch/s390/oprofile/hwsampler.c 2011-05-10 10:53:11.000000000 +0200
@@ -1021,20 +1021,14 @@ deallocate_exit:
return rc;
}
-long hwsampler_query_min_interval(void)
+unsigned long hwsampler_query_min_interval(void)
{
- if (min_sampler_rate)
- return min_sampler_rate;
- else
- return -EINVAL;
+ return min_sampler_rate;
}
-long hwsampler_query_max_interval(void)
+unsigned long hwsampler_query_max_interval(void)
{
- if (max_sampler_rate)
- return max_sampler_rate;
- else
- return -EINVAL;
+ return max_sampler_rate;
}
unsigned long hwsampler_get_sample_overflow_count(unsigned int cpu)
diff -urpN linux-2.6/arch/s390/oprofile/hwsampler.h linux-2.6-patched/arch/s390/oprofile/hwsampler.h
--- linux-2.6/arch/s390/oprofile/hwsampler.h 2011-05-10 10:53:04.000000000 +0200
+++ linux-2.6-patched/arch/s390/oprofile/hwsampler.h 2011-05-10 10:53:11.000000000 +0200
@@ -102,8 +102,8 @@ int hwsampler_setup(void);
int hwsampler_shutdown(void);
int hwsampler_allocate(unsigned long sdbt, unsigned long sdb);
int hwsampler_deallocate(void);
-long hwsampler_query_min_interval(void);
-long hwsampler_query_max_interval(void);
+unsigned long hwsampler_query_min_interval(void);
+unsigned long hwsampler_query_max_interval(void);
int hwsampler_start_all(unsigned long interval);
int hwsampler_stop_all(void);
int hwsampler_deactivate(unsigned int cpu);
diff -urpN linux-2.6/arch/s390/oprofile/init.c linux-2.6-patched/arch/s390/oprofile/init.c
--- linux-2.6/arch/s390/oprofile/init.c 2011-05-10 10:53:04.000000000 +0200
+++ linux-2.6-patched/arch/s390/oprofile/init.c 2011-05-10 10:53:11.000000000 +0200
@@ -145,15 +145,11 @@ static int oprofile_hwsampler_init(struc
* create hwsampler files only if hwsampler_setup() succeeds.
*/
oprofile_min_interval = hwsampler_query_min_interval();
- if (oprofile_min_interval < 0) {
- oprofile_min_interval = 0;
+ if (oprofile_min_interval == 0)
return -ENODEV;
- }
oprofile_max_interval = hwsampler_query_max_interval();
- if (oprofile_max_interval < 0) {
- oprofile_max_interval = 0;
+ if (oprofile_max_interval == 0)
return -ENODEV;
- }
if (oprofile_timer_init(ops))
return -ENODEV;
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
