linux-kernel - Re: [PATCH v4 2/8] nvme-tcp: Update number of hardware queues before using them

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210811102553.auradulozluc5ond@carbon.lan>
Date:   Wed, 11 Aug 2021 12:25:53 +0200
From:   Daniel Wagner <dwagner@...e.de>
To:     Sagi Grimberg <sagi@...mberg.me>
Cc:     Keith Busch <kbusch@...nel.org>, linux-nvme@...ts.infradead.org,
        linux-kernel@...r.kernel.org,
        James Smart <james.smart@...adcom.com>,
        Ming Lei <ming.lei@...hat.com>, Hannes Reinecke <hare@...e.de>,
        Wen Xiong <wenxiong@...ibm.com>
Subject: Re: [PATCH v4 2/8] nvme-tcp: Update number of hardware queues before
 using them

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