Re: [PATCHv2] nvme: always initialize known command effects

[Christoph Hellwig <hch@lst.de>]

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.