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;
From:   sasaki tatsuya <tatsuya6.sasaki@kioxia.com>
To:     Sagi Grimberg <sagi@grimberg.me>,
        "kbusch@kernel.org" <kbusch@kernel.org>,
        "axboe@fb.com" <axboe@fb.com>, "hch@lst.de" <hch@lst.de>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] nvme: update keep alive interval when kato is modified
Thread-Topic: [PATCH v2] nvme: update keep alive interval when kato is
 modified
Thread-Index: AdeYqq0GYCdtylBoR8GOhZA2v9EjoAAMlYwAACq71yA=
Date:   Wed, 25 Aug 2021 08:26:03 +0000
Message-ID: <ad3ea07be9174fb68c06c516192bf05e@kioxia.com>
References: <526a1a756d6c4643b15b1b305cc32817@kioxia.com>
 <05033836-83b9-c060-0348-774a02b60d01@grimberg.me>
In-Reply-To: <05033836-83b9-c060-0348-774a02b60d01@grimberg.me>
Accept-Language: ja-JP, en-US
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
X-CrossPremisesHeadersFilteredBySendConnector: CNN1EXMB03.r1.kioxia.com
X-OrganizationHeadersPreserved: CNN1EXMB03.r1.kioxia.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/25/21 5:42 AM JST, Sagi Grimberg wrote:
> > +	if (cmd->opcode == nvme_admin_set_features &&
> > +	    (cmd->cdw10 & 0xFF) == NVME_FEAT_KATO) {
> > +		/* ms -> s */
> 
> no need for this comment.

Thanks for your comments. I will remove this /* ms -> s*/ comment.

> > +		unsigned int new_kato = DIV_ROUND_UP(cmd->cdw11, 1000);
> > +
> > +		nvme_update_keep_alive(ctrl, new_kato);
> 
> I think you can now inline nvme_update_keep_alive here, no need to keep
> it in a function.

Does this mean the section below needs to be moved from core routine
to nvme_user_cmd_post function?
> > +	dev_info(ctrl->device,
> > +		 "keep alive commands interval on the host is updated from %u ms to %u ms\n",
> > +		 ctrl->kato * 1000 / 2, new_kato * 1000 / 2);
> > +
> > +	nvme_stop_keep_alive(ctrl);
> > +	ctrl->kato = new_kato;
> > +	nvme_start_keep_alive(ctrl);

Thanks.