2022-03-09 00:09:09

by Mike Travis

[permalink] [raw]
Subject: [PATCH 4/4] x86/platform/uv: Add gap hole end size

Show value of gap end in kernel log which equates to number of physical
address bits used by system. The structure stores PA bits 56:26, for
64MB granularity, up to 64PB max size.

Signed-off-by: Mike Travis <[email protected]>
Reviewed-by: Steve Wahl <[email protected]>
---
arch/x86/kernel/apic/x2apic_uv_x.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index 387d6533549a..146f0f63a43b 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -1346,7 +1346,7 @@ static void __init decode_gam_params(unsigned long ptr)
static void __init decode_gam_rng_tbl(unsigned long ptr)
{
struct uv_gam_range_entry *gre = (struct uv_gam_range_entry *)ptr;
- unsigned long lgre = 0;
+ unsigned long lgre = 0, gend = 0;
int index = 0;
int sock_min = 999999, pnode_min = 99999;
int sock_max = -1, pnode_max = -1;
@@ -1380,6 +1380,9 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
flag, size, suffix[order],
gre->type, gre->nasid, gre->sockid, gre->pnode);

+ if (gre->type == UV_GAM_RANGE_TYPE_HOLE)
+ gend = (unsigned long)gre->limit << UV_GAM_RANGE_SHFT;
+
/* update to next range start */
lgre = gre->limit;
if (sock_min > gre->sockid)
@@ -1397,7 +1400,8 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
_max_pnode = pnode_max;
_gr_table_len = index;

- pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x) pnodes(min:%x,max:%x)\n", index, _min_socket, _max_socket, _min_pnode, _max_pnode);
+ pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x), pnodes(min:%x,max:%x), gap_end(%d)\n",
+ index, _min_socket, _max_socket, _min_pnode, _max_pnode, fls64(gend));
}

/* Walk through UVsystab decoding the fields */
--
2.26.2


2022-03-09 01:12:34

by Steve Wahl

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86/platform/uv: Add gap hole end size

Mike,

I know you're trying to get this out and don't really need another
delta, and I'd be holding it back if I didn't think it might make
things smoother upstream.

But what I'd consider for this one is: Add the word log to the
subject line, perhaps "Add gap hole end size to log", or just "Log gap
hole end size". Without it, the reviewer has to ask "add to *where*?"

And I believe the second sentence of the description, "The structure
stores PA bits 56:26, for > 64MB granularity, up to 64PB max size," is
perhaps not necessary, and I think it may slow down somebody trying to
read the patch quickly. So I'd consider deleting it.

With those two changes the description still matches the code, and
seems simpler and easier to accept.

Your call on either / both, of course.

--> Steve

On Mon, Mar 07, 2022 at 07:05:37PM -0600, Mike Travis wrote:
> Show value of gap end in kernel log which equates to number of physical
> address bits used by system. The structure stores PA bits 56:26, for
> 64MB granularity, up to 64PB max size.
>
> Signed-off-by: Mike Travis <[email protected]>
> Reviewed-by: Steve Wahl <[email protected]>
> ---
> arch/x86/kernel/apic/x2apic_uv_x.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
> index 387d6533549a..146f0f63a43b 100644
> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -1346,7 +1346,7 @@ static void __init decode_gam_params(unsigned long ptr)
> static void __init decode_gam_rng_tbl(unsigned long ptr)
> {
> struct uv_gam_range_entry *gre = (struct uv_gam_range_entry *)ptr;
> - unsigned long lgre = 0;
> + unsigned long lgre = 0, gend = 0;
> int index = 0;
> int sock_min = 999999, pnode_min = 99999;
> int sock_max = -1, pnode_max = -1;
> @@ -1380,6 +1380,9 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
> flag, size, suffix[order],
> gre->type, gre->nasid, gre->sockid, gre->pnode);
>
> + if (gre->type == UV_GAM_RANGE_TYPE_HOLE)
> + gend = (unsigned long)gre->limit << UV_GAM_RANGE_SHFT;
> +
> /* update to next range start */
> lgre = gre->limit;
> if (sock_min > gre->sockid)
> @@ -1397,7 +1400,8 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
> _max_pnode = pnode_max;
> _gr_table_len = index;
>
> - pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x) pnodes(min:%x,max:%x)\n", index, _min_socket, _max_socket, _min_pnode, _max_pnode);
> + pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x), pnodes(min:%x,max:%x), gap_end(%d)\n",
> + index, _min_socket, _max_socket, _min_pnode, _max_pnode, fls64(gend));
> }
>
> /* Walk through UVsystab decoding the fields */
> --
> 2.26.2
>

--
Steve Wahl, Hewlett Packard Enterprise

2022-03-11 22:28:46

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 4/4] x86/platform/uv: Add gap hole end size

Hi,

On 3/8/22 17:09, Steve Wahl wrote:
> Mike,
>
> I know you're trying to get this out and don't really need another
> delta, and I'd be holding it back if I didn't think it might make
> things smoother upstream.
>
> But what I'd consider for this one is: Add the word log to the
> subject line, perhaps "Add gap hole end size to log", or just "Log gap
> hole end size". Without it, the reviewer has to ask "add to *where*?"

I agree that "Log gap hole end size" would be a better subject for this patch.

Regards,

Hans


>
> And I believe the second sentence of the description, "The structure
> stores PA bits 56:26, for > 64MB granularity, up to 64PB max size," is
> perhaps not necessary, and I think it may slow down somebody trying to
> read the patch quickly. So I'd consider deleting it.
>
> With those two changes the description still matches the code, and
> seems simpler and easier to accept.
>
> Your call on either / both, of course.
>
> --> Steve
>
> On Mon, Mar 07, 2022 at 07:05:37PM -0600, Mike Travis wrote:
>> Show value of gap end in kernel log which equates to number of physical
>> address bits used by system. The structure stores PA bits 56:26, for
>> 64MB granularity, up to 64PB max size.
>>
>> Signed-off-by: Mike Travis <[email protected]>
>> Reviewed-by: Steve Wahl <[email protected]>
>> ---
>> arch/x86/kernel/apic/x2apic_uv_x.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
>> index 387d6533549a..146f0f63a43b 100644
>> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
>> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
>> @@ -1346,7 +1346,7 @@ static void __init decode_gam_params(unsigned long ptr)
>> static void __init decode_gam_rng_tbl(unsigned long ptr)
>> {
>> struct uv_gam_range_entry *gre = (struct uv_gam_range_entry *)ptr;
>> - unsigned long lgre = 0;
>> + unsigned long lgre = 0, gend = 0;
>> int index = 0;
>> int sock_min = 999999, pnode_min = 99999;
>> int sock_max = -1, pnode_max = -1;
>> @@ -1380,6 +1380,9 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
>> flag, size, suffix[order],
>> gre->type, gre->nasid, gre->sockid, gre->pnode);
>>
>> + if (gre->type == UV_GAM_RANGE_TYPE_HOLE)
>> + gend = (unsigned long)gre->limit << UV_GAM_RANGE_SHFT;
>> +
>> /* update to next range start */
>> lgre = gre->limit;
>> if (sock_min > gre->sockid)
>> @@ -1397,7 +1400,8 @@ static void __init decode_gam_rng_tbl(unsigned long ptr)
>> _max_pnode = pnode_max;
>> _gr_table_len = index;
>>
>> - pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x) pnodes(min:%x,max:%x)\n", index, _min_socket, _max_socket, _min_pnode, _max_pnode);
>> + pr_info("UV: GRT: %d entries, sockets(min:%x,max:%x), pnodes(min:%x,max:%x), gap_end(%d)\n",
>> + index, _min_socket, _max_socket, _min_pnode, _max_pnode, fls64(gend));
>> }
>>
>> /* Walk through UVsystab decoding the fields */
>> --
>> 2.26.2
>>
>