| Message ID | 20200204165535.17214-1-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: fix off-by-one error when printing memory ranges | expand |
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>
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
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
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
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.
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.
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
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
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");
Signed-off-by: Wei Liu <liuwe@microsoft.com> --- xen/arch/x86/e820.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)