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;
IronPort-SDR: +zgTvq1jYazdqFL9i4YBdxgh1Bog+uJb3Oysxn8mUxCgtxI2U4lYQLHuxlS+j9polN7UKhTKNY
 MkWPBDkw2b5LXfCqz+lMC7CmLALGLWvj1MCpe9LlsjCcZlwTyH2JqSWjphkjIn68ZxlCcCgjE2
 up710bLSleEtgYPVFOzHP9NxF63/2Dvd+P1mx1r9L5Vj6F0CoCTVuQcsvJ8E7h2W3/Nkwwjjgh
 lyu9aBdJkVhTErWD2cCytqDty9qVT9IyxaJnBwOSHlpbYFFpjdGYoJQ/sWxKf/sDtmUK3v3DyK
 /zs=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Baolin Wang <baolin.wang@linux.alibaba.com>,
        "kbusch@kernel.org" <kbusch@kernel.org>,
        "axboe@fb.com" <axboe@fb.com>, "hch@lst.de" <hch@lst.de>,
        "sagi@grimberg.me" <sagi@grimberg.me>
CC:     "baolin.wang7@gmail.com" <baolin.wang7@gmail.com>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nvme: Remove redundant validation in nvme_start_ctrl()
Thread-Topic: [PATCH] nvme: Remove redundant validation in nvme_start_ctrl()
Thread-Index: AQHWWN9KvInmw7yU802p8iILluvbIQ==
Date:   Mon, 13 Jul 2020 23:58:18 +0000
Message-ID: <BYAPR04MB49655C05950BC7BDAD30629F86600@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <710eb90d9c1c125bdb37aae31ddc41a0f9782fa6.1594621435.git.baolin.wang@linux.alibaba.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 1127165b-a821-4467-a63a-08d827889d9e
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jul 2020 23:58:18.3952
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: IzbIoHQIBCfmH9/Eim4nLpUMtjFRmCVJ57JXjzS5T9SwCPQ//SvGnpQpZGVExCnKZSxlCD/lw1lsSTsekILZaQR6VSeZI5Crw48Mvw5CqkU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR04MB6549
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 7/12/20 23:31, Baolin Wang wrote:=0A=
> We've already validated the 'kato' in nvme_start_keep_alive(), thus no=0A=
> need to validate it again in nvme_start_ctrl(). Remove it.=0A=
> =0A=
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>=0A=
> ---=0A=
>   drivers/nvme/host/core.c | 3 +--=0A=
>   1 file changed, 1 insertion(+), 2 deletions(-)=0A=
> =0A=
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c=0A=
> index 3d00ea4..a95e26e 100644=0A=
> --- a/drivers/nvme/host/core.c=0A=
> +++ b/drivers/nvme/host/core.c=0A=
> @@ -4313,8 +4313,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)=0A=
>   =0A=
>   void nvme_start_ctrl(struct nvme_ctrl *ctrl)=0A=
>   {=0A=
> -	if (ctrl->kato)=0A=
> -		nvme_start_keep_alive(ctrl);=0A=
> +	nvme_start_keep_alive(ctrl);=0A=
>   =0A=
>   	nvme_enable_aen(ctrl);=0A=
>   =0A=
> =0A=
=0A=
Since start keep alive is so small to make a function call how about we =0A=
in-line the call ? untested patch :-=0A=
=0A=
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c=0A=
index 09abf2ca33f5..c7289c23658a 100644=0A=
--- a/drivers/nvme/host/core.c=0A=
+++ b/drivers/nvme/host/core.c=0A=
@@ -4311,8 +4311,9 @@ EXPORT_SYMBOL_GPL(nvme_stop_ctrl);=0A=
=0A=
  void nvme_start_ctrl(struct nvme_ctrl *ctrl)=0A=
  {=0A=
+       /* if ctrl keep alive time out is set start keep alive */=0A=
         if (ctrl->kato)=0A=
-               nvme_start_keep_alive(ctrl);=0A=
+               queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * =
=0A=
HZ);=0A=
=0A=
         nvme_enable_aen(ctrl);=0A=
=0A=
root@iouring nvme (xarray) # git diff=0A=
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c=0A=
index 09abf2ca33f5..08e1a6826b08 100644=0A=
--- a/drivers/nvme/host/core.c=0A=
+++ b/drivers/nvme/host/core.c=0A=
@@ -1031,14 +1031,6 @@ static void nvme_keep_alive_work(struct =0A=
work_struct *work)=0A=
         }=0A=
  }=0A=
=0A=
-static void nvme_start_keep_alive(struct nvme_ctrl *ctrl)=0A=
-{=0A=
-       if (unlikely(ctrl->kato =3D=3D 0))=0A=
-               return;=0A=
-=0A=
-       queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);=0A=
-}=0A=
-=0A=
  void nvme_stop_keep_alive(struct nvme_ctrl *ctrl)=0A=
  {=0A=
         if (unlikely(ctrl->kato =3D=3D 0))=0A=
@@ -4311,8 +4303,9 @@ EXPORT_SYMBOL_GPL(nvme_stop_ctrl);=0A=
=0A=
  void nvme_start_ctrl(struct nvme_ctrl *ctrl)=0A=
  {=0A=
+       /* if ctrl keep alive time out is set start keep alive */=0A=
         if (ctrl->kato)=0A=
-               nvme_start_keep_alive(ctrl);=0A=
+               queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * =
=0A=
HZ);=0A=
=0A=
         nvme_enable_aen(ctrl);=0A=
=0A=
=0A=