On Wed 15-04-20 20:32:00, Verma, Vishal L wrote:
> On Wed, 2020-04-15 at 12:43 +0200, Michal Hocko wrote:
> > On Tue 14-04-20 17:58:12, Vishal Verma wrote:
> > [...]
> > > +static int check_hotplug_node(int nid)
> > > +{
> > > + int alt_nid;
> > > +
> > > + if (node_possible(nid))
> > > + return nid;
> > > +
> > > + alt_nid = numa_map_to_online_node(nid);
> > > + if (alt_nid == NUMA_NO_NODE)
> > > + alt_nid = first_online_node;
> > > + WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
> > > + "node %d expected, but was absent from the node_possible_map, using %d instead\n",
> > > + nid, alt_nid);
> >
> > I really do not like this. Why should we try to be clever and change the
> > node id requested by the caller? I would just stick with node_possible
> > check and be done with this.
>
> Hi Michal,
>
> Being clever allows us to still use the memory even if it is in a non-
> optimal configuration. Failing here leaves the user no path to add this
> memory until the firmware is fixed. It is the tradeoff between some
> usability vs. how loud we want to be for the failure.
Doing that papers over something that is clearly a FW issue and makes
it "my performance is suboptimal" deal with it OS problem. Really, is
this something we have to care about. Your changelog talks about a Qemu
misconfiguration which is trivial to fix. Has this ever been observed
with a real HW?
--
Michal Hocko
SUSE Labs
