Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.60.0.1.1\))
Subject: Re: [PATCH] ufs: remove an unneed NULL check on list iterator
From:   Jakob Koschel <jakobkoschel@gmail.com>
In-Reply-To: <20220319042657.21835-1-xiam0nd.tong@gmail.com>
Date:   Sun, 20 Mar 2022 01:57:39 +0100
Cc:     jejb@linux.ibm.com, martin.petersen@oracle.com,
        Alim Akhtar <alim.akhtar@samsung.com>, avri.altman@wdc.com,
        daejun7.park@samsung.com, bvanassche@acm.org, beanhuo@micron.com,
        keosung.park@samsung.com, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <6EEE0ACC-6F7F-4D29-AA2B-751E4677B35E@gmail.com>
References: <20220319042657.21835-1-xiam0nd.tong@gmail.com>
To:     Xiaomeng Tong <xiam0nd.tong@gmail.com>
Precedence: bulk


> On 19. Mar 2022, at 05:26, Xiaomeng Tong <xiam0nd.tong@gmail.com> =
wrote:
>=20
> The list iterator is always non-NULL so it doesn't need to be checked.
> Thus just remove the unnecessary NULL check.
>=20
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/scsi/ufs/ufshpb.c | 6 ------
> 1 file changed, 6 deletions(-)
>=20
> diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
> index 2d36a0715..a3db0ad60 100644
> --- a/drivers/scsi/ufs/ufshpb.c
> +++ b/drivers/scsi/ufs/ufshpb.c
> @@ -869,12 +869,6 @@ static struct ufshpb_region =
*ufshpb_victim_lru_info(struct ufshpb_lu *hpb)
> 	struct ufshpb_region *rgn, *victim_rgn =3D NULL;
>=20
> 	list_for_each_entry(rgn, &lru_info->lh_lru_rgn, list_lru_rgn) {
> -		if (!rgn) {
> -			dev_err(&hpb->sdev_ufs_lu->sdev_dev,
> -				"%s: no region allocated\n",
> -				__func__);
> -			return NULL;
> -		}

Maybe it make more sense to move that check after the loop body instead
of removing it completely? This code obviously does nothing in it's =
current
state but it's clear what the original intention was, so restoring that =
would
perhaps be better.

But probably someone with more experience about this code can make the =
call

> 		if (ufshpb_check_srgns_issue_state(hpb, rgn))
> 			continue;
>=20
> --=20
> 2.17.1
>=20

	Jakob