Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 11 Aug 2021 12:25:53 +0200
From:   Daniel Wagner <dwagner@suse.de>
To:     Sagi Grimberg <sagi@grimberg.me>
Cc:     Keith Busch <kbusch@kernel.org>, linux-nvme@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        James Smart <james.smart@broadcom.com>,
        Ming Lei <ming.lei@redhat.com>, Hannes Reinecke <hare@suse.de>,
        Wen Xiong <wenxiong@us.ibm.com>
Subject: Re: [PATCH v4 2/8] nvme-tcp: Update number of hardware queues before
 using them
Message-ID: <20210811102553.auradulozluc5ond@carbon.lan>
References: <20210802112658.75875-1-dwagner@suse.de>
 <20210802112658.75875-3-dwagner@suse.de>
 <8373c07f-f5df-1ec6-9fda-d0262fc1b377@grimberg.me>
 <20210809085250.xguvx5qiv2gxcoqk@carbon>
 <01d7878c-e396-1d6b-c383-8739ca0b3d11@grimberg.me>
 <20210811010718.GA3135947@dhcp-10-100-145-180.wdc.com>
 <079108ce-6ca0-800e-e3df-29d015a4530c@grimberg.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <079108ce-6ca0-800e-e3df-29d015a4530c@grimberg.me>
Precedence: bulk

On Tue, Aug 10, 2021 at 10:57:58PM -0700, Sagi Grimberg wrote:
> > > I think we should always wait for the freeze to complete.
> > 
> > Don't the queues need to be started in order for the freeze to complete?
> > Any enqueued requests on the quiesced queues will never complete this
> > way, so the wait_freeze() will be stuck, right? If so, I think the
> > nvme_start_queues() was in the correct place already.
> 
> Exactly what I was trying to point out (poorly though)

Thanks for explaining. I think I got the general idea what the different
states are doing and what the transitions are now. (famous last words).

Anyway, the first three patches are the result of debugging the case of
'prior_ioq_cnt != nr_io_queues'. Starting the queues before updating the
number of queues lookes strange.

I suppose in the case 'prior_ioq_cnt > nr_io_queues',
nvme_tcp_start_io_queues() should be successful and we do the
blk_mq_update_nr_hw_queues(). In the other case we should land in the
error recovery.

Wouldn't it make sense to always exercise the error recovery path if we
detect 'prior_ioq_cnt != nr_io_queues' and reduce the number of things
which can go wrong?

Daniel