[PATCHv2] nvme: always initialize known command effects

[Keith Busch]

On Mon, Jan 23, 2023 at 05:38:19PM +0100, Christoph Hellwig wrote:
> On Mon, Jan 23, 2023 at 05:35:36PM +0100, Christoph Hellwig wrote:
> > On Mon, Jan 23, 2023 at 09:29:35AM -0700, Keith Busch wrote:
> > > On Mon, Jan 23, 2023 at 03:54:31PM +0530, Kanchan Joshi wrote:
> > > > On Thu, Jan 19, 2023 at 08:41:28AM -0800, Keith Busch wrote:
> > > > > +	if (!ctrl->effects) {
> > > > > +		ctrl->effects = kzalloc(sizeof(*ctrl->effects), GFP_KERNEL);
> > > > 
> > > > ctrl->effects is not getting freed if controller does not support the
> > > > commands-supported-and-effects log page?
> > > 
> > > Right. If the controller doesn't support it, the driver will make one up
> > > with some sane defaults. The point of this patch is that we don't want
> > > to re-construct the defaults for each passthrough command so we need to
> > > store the defaults somewhere.
> > 
> > But it never ends up beeing freed even when the controller is torn
> > down as it never gets added to ctrl->cels.
> 
> And maybe part of that is the weird double storage.  Can we do away
> with ctrl->effects and head->effects and just do the xarray lookup?
> The xarray lookup for our normal case will basically add one more
> pointer chase and one more cache line that is read.

xarray lookups are pretty slow in comparison, though. This is in a fast
path now, so I think we need to keep caching it.

Alternatively, we could replace the xarray with a normal array sized to
highest seen CSI. Realisitically, I don't think we'll see a CSI higher
than 3 any time soon, so it wouldn't take up much space.