Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 25 Jan 2023 10:05:37 -0600 | From | Steve Wahl <> | Subject | Re: [PATCH v2] x86/platform/uv: UV support for sub-NUMA clustering |
| |
On Wed, Jan 25, 2023 at 12:28:07PM +0100, Ingo Molnar wrote: > > * Steve Wahl <steve.wahl@hpe.com> wrote: > > > +static int __init alloc_conv_table(int num_elem, unsigned short **table) > > +{ > > + int i; > > + size_t bytes; > > + > > + bytes = num_elem * sizeof(*table[0]); > > + *table = kmalloc(bytes, GFP_KERNEL); > > + WARN_ON_ONCE(!*table); > > + if (!*table) > > + return -ENOMEM; > > WARN_ON_ONCE() is pass-through on the condition, so you can write this in a > shorter form: > > if (!WARN_ON_ONCE(!*table)) > return -ENOMEM;
That is nicer. I will incorporate this (including all the repeats).
... > > > + WARN_ON_ONCE(!new_hub); > > + if (!new_hub) > > + return; > > Same. Also a memory leak of at least uv_hub_info_list_blade?
You're right, and this is not the only place. I will rework.
... > > + > > + for_each_node(nodeid) { > > + __uv_hub_info_list[nodeid] = uv_hub_info_list_blade[uv_node_to_blade_id(nodeid)]; > > + } > > Unnecessary curly braces.
I will fix.
> Looks good otherwise - presumably it's both tested and backwards compatible > with older UV hardware?
Yes, in fact there was a major re-work before letting it escape to the world because the previous version didn't function on older hardware.
Thank you for taking the time to review!
--> Steve -- Steve Wahl, Hewlett Packard Enterprise
| ![\](/images/icornerr.gif) |