Received-SPF: pass (google.com: domain of linux-kernel+bounces-22838-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Michael Kelley <mhklinux@outlook.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
CC: Markus Elfring <Markus.Elfring@web.de>, "linux-hyperv@vger.kernel.org"
	<linux-hyperv@vger.kernel.org>, "kernel-janitors@vger.kernel.org"
	<kernel-janitors@vger.kernel.org>, Dexuan Cui <decui@microsoft.com>, Haiyang
 Zhang <haiyangz@microsoft.com>, "K. Y. Srinivasan" <kys@microsoft.com>, Wei
 Liu <wei.liu@kernel.org>, "cocci@inria.fr" <cocci@inria.fr>, LKML
	<linux-kernel@vger.kernel.org>
Subject: RE: Drivers: hv: vmbus: One function call less in
 create_gpadl_header() after error detection
Thread-Topic: Drivers: hv: vmbus: One function call less in
 create_gpadl_header() after error detection
Thread-Index: AQHaQ7PyvvrIb8tUYUye7rRKlkvVrLDTNj6ggAAqigCAADt1QA==
Date: Wed, 10 Jan 2024 22:17:17 +0000
Message-ID:
 <SN6PR02MB415710A52835BC82B1FA1EAFD4692@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <6d97cafb-ad7c-41c1-9f20-41024bb18515@web.de>
 <SN6PR02MB4157AA51AD8AEBB24D0668B7D4692@SN6PR02MB4157.namprd02.prod.outlook.com>
 <82054a0a-72e5-45b2-8808-e411a9587406@web.de>
 <SN6PR02MB4157CA3901DD8D069C755C72D4692@SN6PR02MB4157.namprd02.prod.outlook.com>
 <d3c13efc-a1a3-4f19-b0b9-f8c02cc674d5@moroto.mountain>
In-Reply-To: <d3c13efc-a1a3-4f19-b0b9-f8c02cc674d5@moroto.mountain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?owCur0JVW0jvzPDWaN0tI3QKkG2hSWDSwvgoH3imqVS2ObAbQv/hzrYVk5NC?=
 =?us-ascii?Q?6VbuI2OjTlPY5SRX60aaNybj6hTAP6N5oGG6ldcbZJSmTp+RdJJovXu6o16M?=
 =?us-ascii?Q?EED8WkzTG9kctPoKrULEVCxHAvPS+qQhjy//ZcIrOQe8hRC67ojswDLzrknp?=
 =?us-ascii?Q?XDcT+6xbFTdYNq9FyvITq8Qc9mV/NC6HtC5Qu3LLA79PkFwj6sCi8NvPLugo?=
 =?us-ascii?Q?pVFh+Jqs816rcOo3VG+VWC1fwnztyOId7oqpNVcQZlIe7AIWUhNhGxLzGdpv?=
 =?us-ascii?Q?dDg3VQZs+jcYZwuFTOcMfFIBd/Geni2FHTj2RhzDXIdtUZvmL5nmDf3dQv3x?=
 =?us-ascii?Q?SltdDIXR7nZj6MSbuAhMGNV9IVdSJMJGF/2AZUEnujc8HgQ1fOkhYIMt0L3H?=
 =?us-ascii?Q?gHMBKgJb9fM0P60ZOvo/+Jrlg679eg3GBiN7+WGnD0oX22hvJrLtEJuhDTpr?=
 =?us-ascii?Q?/AHnqt/uV98Ri/oUXoKggeMWFYzNzr+UyRVYfRaOfNyYcxZc5ct7T9+5Bxel?=
 =?us-ascii?Q?plUYfx9Pf+R9cImpVnDT+ahfRtDUB57CCfy7rE5Fv148+GwqRoEKTRX9Fb02?=
 =?us-ascii?Q?ueqmnmCv4Lo5OGJxrRZ2RoL7DNdMxRdCD9wWoszoJIS186proCvW/pH8iqJH?=
 =?us-ascii?Q?R5TiReHfyalJ8FE16bSOxXEMQGKuDKRZxHn059rdJx3KI3A91AhwH2/eNEXn?=
 =?us-ascii?Q?0FGHxBCt6ik9Y6L7tnw/UquXoh3CFDmAjpePQpzwG0CZ7HyFEbAb7aZ3eFym?=
 =?us-ascii?Q?CdzSRpzi2Ae5BEycGG1v/lx2a4hV6+B0mbqQxBVjbB8ev8T33geS+S2Ya5x+?=
 =?us-ascii?Q?v0WGlo9lrtR18qlvvu8bHh0/oAgd/h3Ojoq8qthG9uBYDa4ME4P1GB97S9uW?=
 =?us-ascii?Q?J/P/2kj+Auvbkw1/n8AVl6RGR0g56PUm/L1xON+R4hYpul32xXTGy/ewiUJV?=
 =?us-ascii?Q?k7s6KlJ3HXpVZTXa0oVwLoZFUAWnxReEO5ijFrv7OdgdmTrgBSUT4x6lXzFI?=
 =?us-ascii?Q?ZiATV6OatYuULNIErDboP88qFNz3iP3u+GYr71ztFFH5bzJoa4XpvxXv5sFW?=
 =?us-ascii?Q?rriUtX9GCeJiMKXxXtCgmYHYVXUEALtuFFProXzIwfi6wHIXS5NCZhqkWXIv?=
 =?us-ascii?Q?JzJaEUTGdUEaLYGRt0nZwIMzmSkGaktNVVMxpSW/mBE54wvFoYm+UybBNjFR?=
 =?us-ascii?Q?dzA/YL6X+609KAPdwv6CJT+yBgxnyo4FufD+Pw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 21cc1c4c-ba3b-425d-8d1f-08dc1229e7d4
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jan 2024 22:17:17.1966
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR02MB8619

From: Dan Carpenter <dan.carpenter@linaro.org> Sent: Wednesday, January 10,=
 2024 10:38 AM
>=20
> The second half of the if statement is basically duplicated.  It doesn't
> need to be treated as a special case.  We could do something like below.
> I deliberately didn't delete the tabs.  Also I haven't tested it.

Indeed!  I looked at the history, and this function has been
structured with the duplication since sometime in 2010, which
pre-dates my involvement by several years.  I don't know of
any reason why the duplication is needed, and agree it could
be eliminated.

Assuming Markus is OK with my proposal on the handling of
memory allocation failures, a single patch could simplify this
function quite a bit.

Dan -- do you want to create and submit the patch?  I'll test the
code on Hyper-V.  Or I can create, test, and submit the patch with
a "Suggested-by: Dan Carpenter".

Michael

>=20
> regards,
> dan carpenter
>=20
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 56f7e06c673e..2ba65f9ad3f1 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -328,9 +328,9 @@ static int create_gpadl_header(enum hv_gpadl_type typ=
e, void *kbuffer,
>  		  sizeof(struct gpa_range);
>  	pfncount =3D pfnsize / sizeof(u64);
>=20
> -	if (pagecount > pfncount) {
> -		/* we need a gpadl body */
> -		/* fill in the header */
> +	if (pagecount < pfncount)
> +		pfncount =3D pagecount;
> +
>  		msgsize =3D sizeof(struct vmbus_channel_msginfo) +
>  			  sizeof(struct vmbus_channel_gpadl_header) +
>  			  sizeof(struct gpa_range) + pfncount * sizeof(u64);
> @@ -410,31 +410,6 @@ static int create_gpadl_header(enum hv_gpadl_type ty=
pe, void *kbuffer,
>  			pfnsum +=3D pfncurr;
>  			pfnleft -=3D pfncurr;
>  		}
> -	} else {
> -		/* everything fits in a header */
> -		msgsize =3D sizeof(struct vmbus_channel_msginfo) +
> -			  sizeof(struct vmbus_channel_gpadl_header) +
> -			  sizeof(struct gpa_range) + pagecount * sizeof(u64);
> -		msgheader =3D kzalloc(msgsize, GFP_KERNEL);
> -		if (msgheader =3D=3D NULL)
> -			goto nomem;
> -
> -		INIT_LIST_HEAD(&msgheader->submsglist);
> -		msgheader->msgsize =3D msgsize;
> -
> -		gpadl_header =3D (struct vmbus_channel_gpadl_header *)
> -			msgheader->msg;
> -		gpadl_header->rangecount =3D 1;
> -		gpadl_header->range_buflen =3D sizeof(struct gpa_range) +
> -					 pagecount * sizeof(u64);
> -		gpadl_header->range[0].byte_offset =3D 0;
> -		gpadl_header->range[0].byte_count =3D hv_gpadl_size(type, size);
> -		for (i =3D 0; i < pagecount; i++)
> -			gpadl_header->range[0].pfn_array[i] =3D hv_gpadl_hvpfn(
> -				type, kbuffer, size, send_offset, i);
> -
> -		*msginfo =3D msgheader;
> -	}
>=20
>  	return 0;
>  nomem: