2013-03-18 11:11:45

by Lin Feng

[permalink] [raw]
Subject: [PATCH] kernel/range.c: subtract_range: return instead of continue to save some loops

If we fall into that branch it means that there is a range fully covering the
subtract range, so it's suffice to return there if there isn't any other
overlapping ranges.

Also fix the broken phrase issued by printk.

Cc: Yinghai Lu <[email protected]>
Signed-off-by: Lin Feng <[email protected]>
---
kernel/range.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/range.c b/kernel/range.c
index 9b8ae2d..223c6fe 100644
--- a/kernel/range.c
+++ b/kernel/range.c
@@ -97,10 +97,10 @@ void subtract_range(struct range *range, int az, u64 start, u64 end)
range[i].end = range[j].end;
range[i].start = end;
} else {
- printk(KERN_ERR "run of slot in ranges\n");
+ printk(KERN_ERR "run out of slot in ranges\n");
}
range[j].end = start;
- continue;
+ return;
}
}
}
--
1.8.0.1


2013-03-18 17:52:39

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] kernel/range.c: subtract_range: return instead of continue to save some loops

On Mon, Mar 18, 2013 at 3:21 AM, Lin Feng <[email protected]> wrote:
> If we fall into that branch it means that there is a range fully covering the
> subtract range, so it's suffice to return there if there isn't any other
> overlapping ranges.
>
> Also fix the broken phrase issued by printk.
>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Lin Feng <[email protected]>
> ---
> kernel/range.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/range.c b/kernel/range.c
> index 9b8ae2d..223c6fe 100644
> --- a/kernel/range.c
> +++ b/kernel/range.c
> @@ -97,10 +97,10 @@ void subtract_range(struct range *range, int az, u64 start, u64 end)
> range[i].end = range[j].end;
> range[i].start = end;
> } else {
> - printk(KERN_ERR "run of slot in ranges\n");
> + printk(KERN_ERR "run out of slot in ranges\n");

maybe could change to pr_err at the same time.

> }
> range[j].end = start;
> - continue;
> + return;

We don't say that ranges can not be overlapped in the array.

Thanks

Yinghai

2013-03-19 03:52:27

by Lin Feng

[permalink] [raw]
Subject: [PATCH] kernel/range.c: subtract_range: fix the broken phrase issued by printk

Also replace deprecated printk(KERN_ERR...) with pr_err() as suggested
by Yinghai, attaching the function name to provide plenty info.

Cc: Yinghai Lu <[email protected]>
Signed-off-by: Lin Feng <[email protected]>
---
kernel/range.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/range.c b/kernel/range.c
index 9b8ae2d..071b0ab 100644
--- a/kernel/range.c
+++ b/kernel/range.c
@@ -97,7 +97,8 @@ void subtract_range(struct range *range, int az, u64 start, u64 end)
range[i].end = range[j].end;
range[i].start = end;
} else {
- printk(KERN_ERR "run of slot in ranges\n");
+ pr_err("%s: run out of slot in ranges\n",
+ __func__);
}
range[j].end = start;
continue;
--
1.8.0.1

2013-03-27 17:28:04

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] kernel/range.c: subtract_range: fix the broken phrase issued by printk

On Mon, Mar 18, 2013 at 9:54 PM, Lin Feng <[email protected]> wrote:
> Also replace deprecated printk(KERN_ERR...) with pr_err() as suggested
> by Yinghai, attaching the function name to provide plenty info.
>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Lin Feng <[email protected]>
> ---
> kernel/range.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/range.c b/kernel/range.c
> index 9b8ae2d..071b0ab 100644
> --- a/kernel/range.c
> +++ b/kernel/range.c
> @@ -97,7 +97,8 @@ void subtract_range(struct range *range, int az, u64 start, u64 end)
> range[i].end = range[j].end;
> range[i].start = end;
> } else {
> - printk(KERN_ERR "run of slot in ranges\n");
> + pr_err("%s: run out of slot in ranges\n",
> + __func__);
> }
> range[j].end = start;
> continue;

So now the user might see:

subtract_range: run out of slot in ranges

What is the user supposed to do when he sees that? If he happens to
mention it on LKML, what are we going to do about it? If he attaches
the complete dmesg log, is there enough information to do something?

IMHO, that message is still totally useless.

2013-03-27 17:51:58

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] kernel/range.c: subtract_range: fix the broken phrase issued by printk

On Wed, Mar 27, 2013 at 10:27 AM, Bjorn Helgaas <[email protected]> wrote:

> So now the user might see:
>
> subtract_range: run out of slot in ranges
>
> What is the user supposed to do when he sees that? If he happens to
> mention it on LKML, what are we going to do about it? If he attaches
> the complete dmesg log, is there enough information to do something?
>
> IMHO, that message is still totally useless.

Change to WARN_ONCE?

Yinghai

2013-03-28 01:47:58

by Lin Feng

[permalink] [raw]
Subject: Re: [PATCH] kernel/range.c: subtract_range: fix the broken phrase issued by printk

Hi Bjorn and others,

On 03/28/2013 01:27 AM, Bjorn Helgaas wrote:
>> - printk(KERN_ERR "run of slot in ranges\n");
>> > + pr_err("%s: run out of slot in ranges\n",
>> > + __func__);
>> > }
>> > range[j].end = start;
>> > continue;
> So now the user might see:
>
> subtract_range: run out of slot in ranges
>
> What is the user supposed to do when he sees that? If he happens to
> mention it on LKML, what are we going to do about it? If he attaches
> the complete dmesg log, is there enough information to do something?
>
> IMHO, that message is still totally useless.
>

Yes, we need to issue some useful message.
How about dump_stack() there so that we can find some clues more since
subtract_range() is called mtrr_bp_init path and pci relative path, then
it may help to instruct us to do something ;-) ?

thanks,
linfeng