linux-kernel - Re: [PATCH v2] nvme: update keep alive interval when kato is modified

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]

Date:   Wed, 25 Aug 2021 11:04:24 +0200
From:   "hch@....de" <hch@....de>
To:     Sagi Grimberg <sagi@...mberg.me>
Cc:     sasaki tatsuya <tatsuya6.sasaki@...xia.com>,
        "kbusch@...nel.org" <kbusch@...nel.org>,
        "axboe@...com" <axboe@...com>, "hch@....de" <hch@....de>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] nvme: update keep alive interval when kato is
 modified

On Tue, Aug 24, 2021 at 01:41:44PM -0700, Sagi Grimberg wrote:
>> +{
>> +	/*
>> +	 * Keep alive commands interval on the host should be updated
>> +	 * when KATO is modified by Set Features commands.
>> +	 */
>> +	if (cmd->opcode == nvme_admin_set_features &&
>> +	    (cmd->cdw10 & 0xFF) == NVME_FEAT_KATO) {
>> +		/* ms -> s */
>
> no need for this 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.

Actually, іf thinking ahead I think one helper per fixup would be really
useful to keep the code organized.  But the DIV_ROUND_UP should move into
nvme_update_keep_alive to keep it self-contained.
We can then restructure the caller a bit to make it easier to expand:

	switch (cmd->opcode) {
	case nvme_admin_set_features:
		switch (cmd->cdw10 & 0xff) {
		case NVME_FEAT_KATO:
			nvme_update_keep_alive(ctrl, cmd);
			break;
		}
	}