Adding back people on CC, I accidentally reply-to instead of reply-to-all.
On Thu, Jul 12, 2012 at 7:30 PM, Ram Pai <linux...@us.ibm.com> wrote:
> On Thu, Jul 12, 2012 at 12:56:08PM +0300, Purdila, Octavian wrote:
>> On Thu, Jul 12, 2012 at 11:56 AM, Ram Pai <linux...@us.ibm.com> wrote:
>
> ..snip..
>> > Offcourse; it does not warn when the request is partially out of root's
>> > range.
>> > But that should be ok, because its still a valid request.
>>
>> Since in 99% of cases requests should be completely inside the root
>> range (in fact no bug has been reported until now on this pretty old
>> piece of code) I think being verbose here is a good thing.
>>
>> This sorts of problems occur only in exotic setups (in my case it was
>> Xen on a machine with PAE enabled but 32bit address space) and
>> silently adjusting the request will make logical bugs in the upper
>> layers very hard to detect.
>>
>> That being said, I'll leave it to you and Andrew decide which version
>> (verbose or non-verbose) is better.
>>
>> This is the last verbose version which fixes a few issues from last
>> post as well as makes a few cosmetic changes:
>
> this version is correct. I am ok with it. Probably with one minor
> simplication as below.
>>
>> ---
>> kernel/resource.c | 25 ++++++++++++++++++++++++-
>> 1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index e1d2b8e..c0e0fa2 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -7,6 +7,8 @@
>> * Arbitrary resource management.
>> */
>>
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> #include <linux/export.h>
>> #include <linux/errno.h>
>> #include <linux/ioport.h>
>
> these above lines must have crept in by mistake? :)
>
No, this is intentional. Since we now use pr_err I think it useful to
have the messages formatted like:
<3>resource: requested range [0x%llx-0x%llx] not in root %pr
>> @@ -788,8 +790,29 @@ void __init reserve_region_with_split(struct
>> resource *root,
>> resource_size_t start, resource_size_t end,
>> const char *name)
>> {
>> + int abort = 0;
>> +
>> write_lock(&resource_lock);
>> - __reserve_region_with_split(root, start, end, name);
>> + if (!(root->start <= start && root->end >= end)) {
>
> This can be simplified to
> if (root->start > start || root->end < end) {
>
Sure, I will do that. Thanks for reviewing, I will follow up with a v2
patch to Andrew.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
