Received-SPF: pass (google.com: domain of linux-kernel+bounces-31224-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>
To: Jiri Pirko <jiri@resnulli.us>
CC: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"vadim.fedorenko@linux.dev" <vadim.fedorenko@linux.dev>,
	"davem@davemloft.net" <davem@davemloft.net>, "Olech, Milena"
	<milena.olech@intel.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "pabeni@redhat.com" <pabeni@redhat.com>,
	"kuba@kernel.org" <kuba@kernel.org>, mschmidt <mschmidt@redhat.com>, "Glaza,
 Jan" <jan.glaza@intel.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@intel.com>
Subject: RE: [PATCH net v5 2/4] dpll: fix pin dump crash for rebound module
Thread-Topic: [PATCH net v5 2/4] dpll: fix pin dump crash for rebound module
Thread-Index: AQHaSf75qBXfF/1/2kO2cnQNglylbbDfc0oAgAG0xVA=
Date: Fri, 19 Jan 2024 13:49:25 +0000
Message-ID: <DM6PR11MB4657D3968F97531F08BF4F959B702@DM6PR11MB4657.namprd11.prod.outlook.com>
References: <20240118110719.567117-1-arkadiusz.kubalewski@intel.com>
 <20240118110719.567117-3-arkadiusz.kubalewski@intel.com>
 <ZakPBmakH8BTv8Cz@nanopsycho>
In-Reply-To: <ZakPBmakH8BTv8Cz@nanopsycho>
Accept-Language: pl-PL, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/PGIhtvnA6HJs0eTJU9d1miEozpmSDEw/QJZJIKohtc/2/AgOTRX9TZHM2BC?=
 =?us-ascii?Q?i2CF9Q1bJwKIPIfg+ZRunzqdQJy2ShU1VJvGOhUhbOWIqKQEyn1PV51dWiCB?=
 =?us-ascii?Q?b5BQtuQFtC1R7pAdgAMNQpNGJ12/AbcJlPSC7Xhx7mOm6kq+1gSAEhGDA218?=
 =?us-ascii?Q?1cXgYkkk5BjYhSes4A8Z10VdsHdjMGRJSheihor0O1xf2cmX3n1UzMKSNM2z?=
 =?us-ascii?Q?KiJ/tOqODBXGc2bZmXqZEn6lFQnX/AMEn87DOl0GEZLQ6oEbWZkv6mPyzl3E?=
 =?us-ascii?Q?r1xnxvQ8G3FfcozVoLxE7165j3ZAvVeHTZRMQe8O5PRUzu46qlj3rEtJhbvb?=
 =?us-ascii?Q?n9q/jEu8xsTwQ4yoiBZl6c06uXxdgGIoQ+9UWhYBuCt+H64g3VEpTCm7FzvN?=
 =?us-ascii?Q?DGQLhgY0GkyZmLAsElv1D2fz8wumbTkwapawG7R9JC7tdMgXN/GrRfkx8ivi?=
 =?us-ascii?Q?CcKzbfmdkotcELb4KEEjkDxRaafwQT2wkT29vLF9BZxDM7KPgEYJx47q3KYZ?=
 =?us-ascii?Q?ezqfPtEZ4rOxJJEb02FvQ6hpTg4+3HA7R0BC87lx9PSGenzVtaBVUbXdmjsu?=
 =?us-ascii?Q?KPQP7xYRs2kPUE8sUOy3cOtVnmbn0IBo7f1BdIzgY3B1FVqmLie+jWtcxl43?=
 =?us-ascii?Q?LD8n4c9517J6b78wqT4kvpzBBcZ9EgMGmp7FKnK04wLlJ86dLVmMU5ehN3Q3?=
 =?us-ascii?Q?L8D9NjMV1MXUbq/9eXlYi0LHC0lvsWhVOf38bSIazwX2DwY/JQ8aOiWMl+bI?=
 =?us-ascii?Q?rM2qYHuQYcEdoSptJo+KTL9/lQH3iHwztE17qaJuWkUWWHnbhWDV7eMlDotF?=
 =?us-ascii?Q?F+52eHXuiF2w1Y6lH5/jlZH4yTPWhcTtWsHSbiZTNAgyZO7CMOxRjR9cOfJw?=
 =?us-ascii?Q?LVBj2j8aqsW3iFMEHlBoM8um+7EAv5ikZiRCyL7iuT7ZZR4GdqfsOwaYNLne?=
 =?us-ascii?Q?6SJbs+MIxQXHq2bygy4P6ktlwSbo50cVLpRyG5asrIiVVO/aHe9Cycram0V/?=
 =?us-ascii?Q?3ZqezbLV7SM+N+VD6jEoXck9zVKCaLtz4/gppnCxx39sXMMsRCR/C2U9we/i?=
 =?us-ascii?Q?XuwcuiG5VtZfhPLdei6cY6q5dkDrkG5TdhCZaOL3HhIpAbyUSf/yfIfAhehn?=
 =?us-ascii?Q?Rx9eZ6G2Pyg+1B5mfDPtNaNY57MRUAfa6kFAnGffzYPV9Cj5ZPNrYk87OSmw?=
 =?us-ascii?Q?P3C3Fd9WH3klaum0kwqIKERiyEW9Hf+SAXoESQbEYhqk4dXmkRLOKWu11U8+?=
 =?us-ascii?Q?3TtYc2B4AcHUiy7eHq/NoP5HmshzaR5cRj9k8WZnsnayipV/8RpXNRH4JJYZ?=
 =?us-ascii?Q?psICyFOMEAo7bKkPUcpZswwMzyV034Dw2pAUX/kge8E5nr0+KYeSUJPvApHF?=
 =?us-ascii?Q?7icziUfnQ1KeOloj0eWJeeklDqqWCqnDXR9fObWN9Q0TwCNdISqYwEKE5jwW?=
 =?us-ascii?Q?/iphWTh3QBv4upDExxAjpq1xhCHxAyXb8/BZqt0lomIDxLaHQ12V/IjEj04+?=
 =?us-ascii?Q?djbhgrgOr94q0ZMbD1ErSeBOp3qWtT4Cl+J4xfh900isPXXG9IKDghDIkjTW?=
 =?us-ascii?Q?9SKsZ0T8veD3OPKHJJ2UXfqow0os6w93SxDyl/xU8jwdDHeDqa/po7r7mX3S?=
 =?us-ascii?Q?9g=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-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4657.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bf8a32b1-2da5-4e01-ae58-08dc18f57322
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jan 2024 13:49:25.7029
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: DXR3EASVd55mYgUkIvD8VLgwxdlD7iF61ZdzilD870KqvpiRSJVzbiL6VkoJjxvci6PIOAVuo2Dprbq7uXW2VnacgUA6hq6F9HfDfjMmtiI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7786
X-OriginatorOrg: intel.com

>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Thursday, January 18, 2024 12:44 PM
>
>Thu, Jan 18, 2024 at 12:07:17PM CET, arkadiusz.kubalewski@intel.com wrote:
>
>[...]
>
>>@@ -443,7 +490,9 @@ dpll_pin_alloc(u64 clock_id, u32 pin_idx, struct
>>module *module,
>> 		ret =3D -EINVAL;
>> 		goto err_pin_prop;
>> 	}
>>-	pin->prop =3D prop;
>>+	ret =3D dpll_pin_prop_dup(prop, &pin->prop);
>>+	if (ret)
>>+		goto err_pin_prop;
>> 	refcount_set(&pin->refcount, 1);
>> 	xa_init_flags(&pin->dpll_refs, XA_FLAGS_ALLOC);
>> 	xa_init_flags(&pin->parent_refs, XA_FLAGS_ALLOC);
>
>You are missing dpll_pin_prop_free() call on error path. It should go
>right above "err_pin_prop:" line.
>
>Haste makes waste..


True, fixed in v6.

>
>pw-bot: cr
>
>
>
>>@@ -515,6 +564,7 @@ void dpll_pin_put(struct dpll_pin *pin)
>> 		xa_destroy(&pin->dpll_refs);
>> 		xa_destroy(&pin->parent_refs);
>> 		xa_erase(&dpll_pin_xa, pin->id);
>>+		dpll_pin_prop_free(&pin->prop);
>
>To be symmetric with dpll_pin_alloc() order, xa_erase() should be called
>first here and xa_destroys() in different order. But that is a material
>for net-next.

Makes sense.

Thank you!
Arkadiusz

>
>
>
>> 		kfree(pin);
>> 	}
>> 	mutex_unlock(&dpll_lock);
>
>[...]