[Severity: Low]
Does moving the ice_vsi_realloc_stat_arrays() call change what happens when
the stat array allocation fails during a rebuild?
Before this patch, ice_vsi_rebuild() did the realloc before
ice_vsi_decfg(), so a krealloc_array() failure returned -ENOMEM with the VSI
still fully configured:
ret = ice_vsi_realloc_stat_arrays(vsi);
if (ret)
goto unlock;
ice_vsi_decfg(vsi);
Now the only call site is inside ice_vsi_cfg_def(), which runs after
ice_vsi_decfg(), so an -ENOMEM there unwinds via unroll_vsi_alloc and is
propagated out of ice_vsi_rebuild() with the queues, vectors and rings
already released.
For the ethtool -L path that leaves ice_vsi_recfg_qs() in ice_main.c with
only this recovery:
rebuild_err:
dev_err(ice_pf_to_dev(pf), "Error during VSI rebuild: %d. Unload and
reload the driver.\n",
err);
The same outcome was already reachable pre-patch through the other
allocations in ice_vsi_cfg_def() (ice_vsi_alloc_def(),
ice_vsi_alloc_q_vectors(), ice_vsi_alloc_rings(),
ice_vsi_alloc_ring_stats()), so this adds one more allocation to an
already-destructive region rather than a new failure mode.
The commit message describes the move only as:
Also drop one callsite to avoid some duplication.
Could it also mention that the previously non-destructive failure path for
this allocation is gone?
I see no point documenting such minor details in commit message.
As a whole, this patch trades a real bug - memory overwrite - for
a slightly different order of operations and slightly different
failure point when there is no memory
> --
> pw-bot: cr
I would still like to apply as-is, it would be better to spent
"refactor time" on different things in this driver.
