Hi Rob,
On Mon, 4 Mar 2024 09:22:02 -0600 Rob Herring robh@kernel.org wrote:
...
@@ -853,6 +854,14 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) { int i;
- /*
* Wait for any ongoing device link removals before removing some of* nodes. Drop the global lock while waiting*/- mutex_unlock(&of_mutex);
- device_link_wait_removal();
- mutex_lock(&of_mutex);
I'm still not convinced we need to drop the lock. What happens if someone else grabs the lock while we are in device_link_wait_removal()? Can we guarantee that we can't screw things badly?
It is also just ugly because it's the callers of free_overlay_changeset() that hold the lock and now we're releasing it behind their back.
As device_link_wait_removal() is called before we touch anything, can't it be called before we take the lock? And do we need to call it if applying the overlay fails?
Indeed, having device_link_wait_removal() is not needed when applying the overlay fails.
I can call device_link_wait_removal() from the caller of_overlay_remove() but not before the lock is taken. We need to call it between __of_changeset_revert_notify() and free_overlay_changeset() and so, the lock is taken.
This lead to the following sequence: --- 8< --- int of_overlay_remove(int *ovcs_id) { ... mutex_lock(&of_mutex); ...
ret = __of_changeset_revert_notify(&ovcs->cset); ...
ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); ...
mutex_unlock(&of_mutex); device_link_wait_removal(); mutex_lock(&of_mutex);
free_overlay_changeset(ovcs); ... mutex_unlock(&of_mutex); ... } --- 8< ---
In this sequence, the question is: Do we need to release the mutex lock while device_link_wait_removal() is called ?
Best regards, Hervé