Re: [PATCH net-next 10/10] dpll: document locking rules

[Petr Oros <poros@redhat.com>]

On 11/6/26 1:10 PM, Ivan Vecera wrote:
> Describe the lock levels of the subsystem, which domain a callback runs
> under and the rules device drivers have to follow.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   Documentation/driver-api/dpll.rst | 44 +++++++++++++++++++++++++++++++
>   drivers/dpll/dpll_core.c          |  9 +++++++
>   2 files changed, 53 insertions(+)
> 
> diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
> index 7c117ae37cc1bc..40d5037ff3bb0f 100644
> --- a/Documentation/driver-api/dpll.rst
> +++ b/Documentation/driver-api/dpll.rst
> @@ -675,6 +675,50 @@ In the error path we have to rewind every allocation in the reverse order:
>   
>   More complex example can be found in Intel's ICE driver or nVidia's mlx5 driver.
>   
> +Locking
> +=======
> +
> +The subsystem uses two levels of locks, always taken in this order:
> +
> +- ``dpll_lock`` protects the topology of dpll devices and pins and their
> +  lifetime. Functions changing the topology, for example dpll_pin_register(),
> +  take it for write and send the create and delete notifications under it.
> +  Netlink requests, each part of a dump and change notifications take it for
> +  read for their whole duration.
> +- A domain lock serializes driver callbacks. All dpll devices and pins created
> +  with the same module and ``clock_id`` belong to one domain, in practice to
> +  one chip or card.
> +
> +A netlink request addressing a device or a pin by its ID holds the domain lock
> +of that object from start to end. A notification holds it while its message is
> +built and sent, so messages of one object are sent in the order they were
> +built. Only one domain lock is held at a time, so domains have no lock order,
> +and callbacks of different domains can run concurrently.
> +
> +Callbacks of a dpll device run under the domain of the device. Callbacks of a
> +pin run under the domain of the pin, whichever driver registered them:
> +
> +- A driver registering a pin of another driver with its dpll device has these
> +  callbacks serialized with the driver that created the pin, not with the other
> +  callbacks of its device, so it has to protect the state they share by itself.
> +  A callback stuck in such a registration stops the domain of the pin.
> +- The parent pin of a pin registered with dpll_pin_on_pin_register() and its
> +  dpll devices can belong to another driver, so the callback must not
> +  dereference their private data.
> +- The frequency monitor belongs to the dpll device, so only callbacks of a pin
> +  of the same domain registered with dpll_pin_register() can include
> +  ``.measured_freq_get``.
> +
> +Device drivers have to follow these rules:
> +
> +- A callback must not call dpll_device_change_ntf(), dpll_pin_change_ntf() or
> +  any function changing the topology.
> +- A callback notifies about objects of the domain it runs under with
> +  __dpll_device_change_ntf() or __dpll_pin_change_ntf(). Other objects have to
> +  be notified from a context that holds no domain lock, for example a work item.
> +- A driver must not call dpll_device_change_ntf(), dpll_pin_change_ntf() or any
> +  function changing the topology while holding a lock that its callbacks take.
> +
>   SyncE enablement
>   ================
>   For SyncE enablement it is required to allow control over dpll device
> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
> index 84377f8ba16036..6ad40fb9eee855 100644
> --- a/drivers/dpll/dpll_core.c
> +++ b/drivers/dpll/dpll_core.c
> @@ -932,6 +932,10 @@ __dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
>    * @ops: ops for a dpll pin ops
>    * @priv: pointer to private information of owner
>    *
> + * The callbacks run under the domain of @pin. If @pin belongs to another domain
> + * than @dpll, they are not serialized with the callbacks of @dpll and @ops must
> + * not include .measured_freq_get.
> + *
>    * Context: Acquires dpll_lock and domain locks
>    * Return:
>    * * 0 on success
> @@ -1049,6 +1053,11 @@ EXPORT_SYMBOL_GPL(dpll_pin_unregister);
>    * Register a pin with a parent pin, create references between them and
>    * between newly registered pin and dplls connected with a parent pin.
>    *
> + * The callbacks run under the domain of @pin. The parent pin and its dplls can
> + * belong to another driver, so @ops must not dereference their private data.
> + * The frequency monitor belongs to a dpll, so @ops must not include
> + * .measured_freq_get.
> + *
>    * Context: Acquires dpll_lock and domain locks
>    * Return:
>    * * 0 on success
Reviewed-by: Petr Oros <poros@redhat.com>