Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <083e29ff-4ad0-d05f-0521-56ace13629e6@grimberg.me>
Date:   Wed, 21 Jun 2023 12:50:29 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH blktests v1 1/3] nvme/048: Check for queue count check
 directly
Content-Language: en-US
To:     Daniel Wagner <dwagner@suse.de>
Cc:     linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-block@vger.kernel.org, Chaitanya Kulkarni <kch@nvidia.com>,
        Shin'ichiro Kawasaki <shinichiro@fastmail.com>,
        Hannes Reinecke <hare@suse.de>,
        James Smart <jsmart2021@gmail.com>,
        Martin Belanger <Martin.Belanger@dell.com>
References: <20230620132703.20648-1-dwagner@suse.de>
 <20230620132703.20648-2-dwagner@suse.de>
 <380cde65-8794-cfbc-237f-30a8d7e9330c@grimberg.me>
 <f6rvvhgim4nk34onfgbwyfctgvgdbdigwbsugjvizgtli46p57@s7c4gkqrpjqy>
From:   Sagi Grimberg <sagi@grimberg.me>
In-Reply-To: <f6rvvhgim4nk34onfgbwyfctgvgdbdigwbsugjvizgtli46p57@s7c4gkqrpjqy>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


>>> +nvmf_wait_for_queue_count() {
>>> +	local subsys_name="$1"
>>> +	local queue_count="$2"
>>> +	local nvmedev
>>> +
>>> +	nvmedev=$(_find_nvme_dev "${subsys_name}")
>>> +
>>> +	queue_count_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/queue_count"
>>> +
>>> +	nvmf_wait_for_state "${subsys_name}" "live" || return 1
>>> +
>>> +	queue_count=$((queue_count + 1))
>>> +	if grep -q "${queue_count}" "${queue_count_file}"; then
>>> +		return 0
>>> +	fi
>>> +
>>> +	echo "expected queue count ${queue_count} not set"
>>> +	return 1
>>> +}
>>> +
>>>    set_nvmet_attr_qid_max() {
>>>    	local nvmet_subsystem="$1"
>>>    	local qid_max="$2"
>>> @@ -56,10 +76,7 @@ set_qid_max() {
>>>    	local qid_max="$3"
>>>    	set_nvmet_attr_qid_max "${subsys_name}" "${qid_max}"
>>> -
>>> -	# Setting qid_max forces a disconnect and the reconntect attempt starts
>>> -	nvmf_wait_for_state "${subsys_name}" "connecting" || return 1
>>> -	nvmf_wait_for_state "${subsys_name}" "live" || return 1
>>> +	nvmf_wait_for_queue_count "${subsys_name}" "${qid_max}" || return 1
>>
>> Why not simply wait for live? The connecting is obviously racy...
> 
> That is what the new version is doing. It's waiting for the live state and then
> checks the queue count.

Maybe don't fold waiting for live into waiting for queue_count.