Re: [PATCH v7 03/23] firmware: arm_scmi: Introduce protocol instance notifiers

Hi Cristian,

A couple of notifier lifetime issues inline.

> 	scoped_guard(mutex, &info->protocols_mtx) {
> 		pi = idr_find(&info->protocols, protocol_id);
> 		if (WARN_ON(!pi))
> 			return;
>
> 		proto_notifier_nb = pi->pno.nb;
> 		/* Ensure NULL is visible */
> 		smp_store_mb(pi->pno.nb, NULL);
> 	}
>
> 	if (proto_notifier_nb) {
> 		int ret;
>
> 		ret = scmi_protocol_notifier_unregister(pi->handle,
> 							&pi->pno);
> 		if (ret)
> 			dev_err(handle->dev,
> 				"Failed to release protocol notifier\n");
> 	}
>
> 	guard(mutex)(&info->protocols_mtx);
> 	if (refcount_dec_and_test(&pi->users)) {

The notifier is cleared and unregistered before decrementing the protocol
users refcount. If the protocol instance has multiple users, the first
user that releases it removes the protocol implementation's notifier even
though the instance remains active for the remaining users.

Should notifier removal happen only when the final protocol reference is
released?

The ordering may need to be reworked so the final-reference decision and
clearing of pno are made under protocols_mtx, while the potentially
blocking notifier unregister operation is performed without freeing the
protocol instance underneath it.

> 	if (proto_notifier_nb) {
> 		int ret;
>
> 		ret = scmi_protocol_notifier_register(pi->handle, &pi->pno);
> 		if (ret)
> 			dev_warn(handle->dev,
> 				 "Failed to register protocol notifier\n");
> 	}

If registration fails, pi->pno.nb remains populated. Future acquisitions
of the existing protocol instance do not retry registration, while release
later attempts to unregister the notifier even though registration never
succeeded.

Please either clear the stored notifier on registration failure or track
registration state separately and provide a defined retry/error path.

Thanks,
Fayçal