[Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Wei Liu posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200204165535.17214-1-liuwe@microsoft.com
xen/arch/x86/e820.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

[Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Wei Liu 1 week ago
Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/arch/x86/e820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index b9f589cac3..d67387f137 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
     for (i = 0; i < entries; i++) {
         printk(" %016Lx - %016Lx ",
                (unsigned long long)(map[i].addr),
-               (unsigned long long)(map[i].addr + map[i].size));
+               (unsigned long long)(map[i].addr + map[i].size) - 1);
         switch (map[i].type) {
         case E820_RAM:
             printk("(usable)\n");
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Jan Beulich 1 week ago
On 04.02.2020 17:55, Wei Liu wrote:
> Signed-off-by: Wei Liu <liuwe@microsoft.com>
> ---
>  xen/arch/x86/e820.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> index b9f589cac3..d67387f137 100644
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
>      for (i = 0; i < entries; i++) {
>          printk(" %016Lx - %016Lx ",
>                 (unsigned long long)(map[i].addr),
> -               (unsigned long long)(map[i].addr + map[i].size));
> +               (unsigned long long)(map[i].addr + map[i].size) - 1);

Why was this an error? If we used [,] like Linux does - sure.
But we don't. The presentation, without looking at the source,
simply leaves open whether this was meant to be [,] or [,).
And it continues to be left open with the adjustment made.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Wei Liu 1 week ago
On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
> On 04.02.2020 17:55, Wei Liu wrote:
> > Signed-off-by: Wei Liu <liuwe@microsoft.com>
> > ---
> >  xen/arch/x86/e820.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> > index b9f589cac3..d67387f137 100644
> > --- a/xen/arch/x86/e820.c
> > +++ b/xen/arch/x86/e820.c
> > @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
> >      for (i = 0; i < entries; i++) {
> >          printk(" %016Lx - %016Lx ",
> >                 (unsigned long long)(map[i].addr),
> > -               (unsigned long long)(map[i].addr + map[i].size));
> > +               (unsigned long long)(map[i].addr + map[i].size) - 1);
> 
> Why was this an error? If we used [,] like Linux does - sure.
> But we don't. The presentation, without looking at the source,
> simply leaves open whether this was meant to be [,] or [,).
> And it continues to be left open with the adjustment made.
> 

Well, Linux's representation is not what is normally done in math
either.

It is like

  Xen: [mem 0x0000000000000000-0x000000000009efff] usable

Note it is using '-', not ','. And there is "mem" at the beginning.

I have always interpreted the [] pair as something to enclose the range,
not of mathematically meaning.

If you want, I can change Xen's format string to "[%016Lx, %016Lx]".

Wei.


> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Jan Beulich 1 week ago
On 04.02.2020 18:19, Wei Liu wrote:
> On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
>> On 04.02.2020 17:55, Wei Liu wrote:
>>> Signed-off-by: Wei Liu <liuwe@microsoft.com>
>>> ---
>>>  xen/arch/x86/e820.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
>>> index b9f589cac3..d67387f137 100644
>>> --- a/xen/arch/x86/e820.c
>>> +++ b/xen/arch/x86/e820.c
>>> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
>>>      for (i = 0; i < entries; i++) {
>>>          printk(" %016Lx - %016Lx ",
>>>                 (unsigned long long)(map[i].addr),
>>> -               (unsigned long long)(map[i].addr + map[i].size));
>>> +               (unsigned long long)(map[i].addr + map[i].size) - 1);
>>
>> Why was this an error? If we used [,] like Linux does - sure.
>> But we don't. The presentation, without looking at the source,
>> simply leaves open whether this was meant to be [,] or [,).
>> And it continues to be left open with the adjustment made.
>>
> 
> Well, Linux's representation is not what is normally done in math
> either.
> 
> It is like
> 
>   Xen: [mem 0x0000000000000000-0x000000000009efff] usable
> 
> Note it is using '-', not ','. And there is "mem" at the beginning.
> 
> I have always interpreted the [] pair as something to enclose the range,
> not of mathematically meaning.
> 
> If you want, I can change Xen's format string to "[%016Lx, %016Lx]".

I think this would make things less ambiguous, yes. But my primary
request here is to have neither "fix" nor "error" nor anything
alike in the title or description.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Wei Liu 1 week ago
On Wed, Feb 05, 2020 at 09:12:50AM +0100, Jan Beulich wrote:
> On 04.02.2020 18:19, Wei Liu wrote:
> > On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
> >> On 04.02.2020 17:55, Wei Liu wrote:
> >>> Signed-off-by: Wei Liu <liuwe@microsoft.com>
> >>> ---
> >>>  xen/arch/x86/e820.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> >>> index b9f589cac3..d67387f137 100644
> >>> --- a/xen/arch/x86/e820.c
> >>> +++ b/xen/arch/x86/e820.c
> >>> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
> >>>      for (i = 0; i < entries; i++) {
> >>>          printk(" %016Lx - %016Lx ",
> >>>                 (unsigned long long)(map[i].addr),
> >>> -               (unsigned long long)(map[i].addr + map[i].size));
> >>> +               (unsigned long long)(map[i].addr + map[i].size) - 1);
> >>
> >> Why was this an error? If we used [,] like Linux does - sure.
> >> But we don't. The presentation, without looking at the source,
> >> simply leaves open whether this was meant to be [,] or [,).
> >> And it continues to be left open with the adjustment made.
> >>
> > 
> > Well, Linux's representation is not what is normally done in math
> > either.
> > 
> > It is like
> > 
> >   Xen: [mem 0x0000000000000000-0x000000000009efff] usable
> > 
> > Note it is using '-', not ','. And there is "mem" at the beginning.
> > 
> > I have always interpreted the [] pair as something to enclose the range,
> > not of mathematically meaning.
> > 
> > If you want, I can change Xen's format string to "[%016Lx, %016Lx]".
> 
> I think this would make things less ambiguous, yes. But my primary
> request here is to have neither "fix" nor "error" nor anything
> alike in the title or description.

OK. I can certainly change the subject line to

  x86: subtract 1 when printing e820 ranges

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Wei Liu 1 week ago
On Wed, Feb 05, 2020 at 11:45:39AM +0000, Wei Liu wrote:
> On Wed, Feb 05, 2020 at 09:12:50AM +0100, Jan Beulich wrote:
> > On 04.02.2020 18:19, Wei Liu wrote:
> > > On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
> > >> On 04.02.2020 17:55, Wei Liu wrote:
> > >>> Signed-off-by: Wei Liu <liuwe@microsoft.com>
> > >>> ---
> > >>>  xen/arch/x86/e820.c | 2 +-
> > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> > >>> index b9f589cac3..d67387f137 100644
> > >>> --- a/xen/arch/x86/e820.c
> > >>> +++ b/xen/arch/x86/e820.c
> > >>> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
> > >>>      for (i = 0; i < entries; i++) {
> > >>>          printk(" %016Lx - %016Lx ",
> > >>>                 (unsigned long long)(map[i].addr),
> > >>> -               (unsigned long long)(map[i].addr + map[i].size));
> > >>> +               (unsigned long long)(map[i].addr + map[i].size) - 1);
> > >>
> > >> Why was this an error? If we used [,] like Linux does - sure.
> > >> But we don't. The presentation, without looking at the source,
> > >> simply leaves open whether this was meant to be [,] or [,).
> > >> And it continues to be left open with the adjustment made.
> > >>
> > > 
> > > Well, Linux's representation is not what is normally done in math
> > > either.
> > > 
> > > It is like
> > > 
> > >   Xen: [mem 0x0000000000000000-0x000000000009efff] usable
> > > 
> > > Note it is using '-', not ','. And there is "mem" at the beginning.
> > > 
> > > I have always interpreted the [] pair as something to enclose the range,
> > > not of mathematically meaning.
> > > 
> > > If you want, I can change Xen's format string to "[%016Lx, %016Lx]".
> > 
> > I think this would make things less ambiguous, yes. But my primary
> > request here is to have neither "fix" nor "error" nor anything
> > alike in the title or description.
> 
> OK. I can certainly change the subject line to
> 
>   x86: subtract 1 when printing e820 ranges

If I hear no further objections I will commit this patch with the above
subject line today.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Jan Beulich 1 week ago
On 06.02.2020 12:34, Wei Liu wrote:
> On Wed, Feb 05, 2020 at 11:45:39AM +0000, Wei Liu wrote:
>> On Wed, Feb 05, 2020 at 09:12:50AM +0100, Jan Beulich wrote:
>>> On 04.02.2020 18:19, Wei Liu wrote:
>>>> On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
>>>>> On 04.02.2020 17:55, Wei Liu wrote:
>>>>>> Signed-off-by: Wei Liu <liuwe@microsoft.com>
>>>>>> ---
>>>>>>  xen/arch/x86/e820.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
>>>>>> index b9f589cac3..d67387f137 100644
>>>>>> --- a/xen/arch/x86/e820.c
>>>>>> +++ b/xen/arch/x86/e820.c
>>>>>> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
>>>>>>      for (i = 0; i < entries; i++) {
>>>>>>          printk(" %016Lx - %016Lx ",
>>>>>>                 (unsigned long long)(map[i].addr),
>>>>>> -               (unsigned long long)(map[i].addr + map[i].size));
>>>>>> +               (unsigned long long)(map[i].addr + map[i].size) - 1);
>>>>>
>>>>> Why was this an error? If we used [,] like Linux does - sure.
>>>>> But we don't. The presentation, without looking at the source,
>>>>> simply leaves open whether this was meant to be [,] or [,).
>>>>> And it continues to be left open with the adjustment made.
>>>>>
>>>>
>>>> Well, Linux's representation is not what is normally done in math
>>>> either.
>>>>
>>>> It is like
>>>>
>>>>   Xen: [mem 0x0000000000000000-0x000000000009efff] usable
>>>>
>>>> Note it is using '-', not ','. And there is "mem" at the beginning.
>>>>
>>>> I have always interpreted the [] pair as something to enclose the range,
>>>> not of mathematically meaning.
>>>>
>>>> If you want, I can change Xen's format string to "[%016Lx, %016Lx]".
>>>
>>> I think this would make things less ambiguous, yes. But my primary
>>> request here is to have neither "fix" nor "error" nor anything
>>> alike in the title or description.
>>
>> OK. I can certainly change the subject line to
>>
>>   x86: subtract 1 when printing e820 ranges
> 
> If I hear no further objections I will commit this patch with the above
> subject line today.

And with the presentation changed to [,]?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Wei Liu 1 week ago
On Thu, Feb 06, 2020 at 12:38:37PM +0100, Jan Beulich wrote:
> On 06.02.2020 12:34, Wei Liu wrote:
> > On Wed, Feb 05, 2020 at 11:45:39AM +0000, Wei Liu wrote:
> >> On Wed, Feb 05, 2020 at 09:12:50AM +0100, Jan Beulich wrote:
> >>> On 04.02.2020 18:19, Wei Liu wrote:
> >>>> On Tue, Feb 04, 2020 at 06:07:00PM +0100, Jan Beulich wrote:
> >>>>> On 04.02.2020 17:55, Wei Liu wrote:
> >>>>>> Signed-off-by: Wei Liu <liuwe@microsoft.com>
> >>>>>> ---
> >>>>>>  xen/arch/x86/e820.c | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
> >>>>>> index b9f589cac3..d67387f137 100644
> >>>>>> --- a/xen/arch/x86/e820.c
> >>>>>> +++ b/xen/arch/x86/e820.c
> >>>>>> @@ -94,7 +94,7 @@ static void __init print_e820_memory_map(struct e820entry *map, unsigned int ent
> >>>>>>      for (i = 0; i < entries; i++) {
> >>>>>>          printk(" %016Lx - %016Lx ",
> >>>>>>                 (unsigned long long)(map[i].addr),
> >>>>>> -               (unsigned long long)(map[i].addr + map[i].size));
> >>>>>> +               (unsigned long long)(map[i].addr + map[i].size) - 1);
> >>>>>
> >>>>> Why was this an error? If we used [,] like Linux does - sure.
> >>>>> But we don't. The presentation, without looking at the source,
> >>>>> simply leaves open whether this was meant to be [,] or [,).
> >>>>> And it continues to be left open with the adjustment made.
> >>>>>
> >>>>
> >>>> Well, Linux's representation is not what is normally done in math
> >>>> either.
> >>>>
> >>>> It is like
> >>>>
> >>>>   Xen: [mem 0x0000000000000000-0x000000000009efff] usable
> >>>>
> >>>> Note it is using '-', not ','. And there is "mem" at the beginning.
> >>>>
> >>>> I have always interpreted the [] pair as something to enclose the range,
> >>>> not of mathematically meaning.
> >>>>
> >>>> If you want, I can change Xen's format string to "[%016Lx, %016Lx]".
> >>>
> >>> I think this would make things less ambiguous, yes. But my primary
> >>> request here is to have neither "fix" nor "error" nor anything
> >>> alike in the title or description.
> >>
> >> OK. I can certainly change the subject line to
> >>
> >>   x86: subtract 1 when printing e820 ranges
> > 
> > If I hear no further objections I will commit this patch with the above
> > subject line today.
> 
> And with the presentation changed to [,]?

Fine by me too. I will post a new patch shortly.

Wei.

> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86: fix off-by-one error when printing memory ranges

Posted by Andrew Cooper 1 week ago
On 04/02/2020 16:55, Wei Liu wrote:
> Signed-off-by: Wei Liu <liuwe@microsoft.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel