DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0FD2A2199D
MIME-Version: 1.0
In-Reply-To: <1512402456-8176-3-git-send-email-geert+renesas@glider.be>
References: <1512402456-8176-1-git-send-email-geert+renesas@glider.be> <1512402456-8176-3-git-send-email-geert+renesas@glider.be>
From: Rob Herring <robh+dt@kernel.org>
Date: Mon, 4 Dec 2017 13:35:30 -0600
Message-ID: <CAL_JsqJqb-01rQ6D65-r3Q1ZdF407b11BrVgnUQZPjyTW8hj8A@mail.gmail.com>
Subject: Re: [PATCH v2 2/2] of: overlay: Fix cleanup order in of_overlay_apply()
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>,
        Frank Rowand <frowand.list@gmail.com>,
        Colin King <colin.king@canonical.com>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 828
Lines: 21

On Mon, Dec 4, 2017 at 9:47 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The special overlay mutex is taken first, hence it should be released
> last in the error path.
>
> Move "mutex_lock(&of_mutex)" up, as suggested by Frank, as
> free_overlay_changeset() should be called with that mutex held if any
> non-trivial cleanup is to be done.

Not holding the of_mutex for of_resolve_phandles is just wrong.
Without it, a node and new phandle could be added via of_attach_node
making the max phandle wrong.

Now, with the 2 mutexes adjacent, what is the point of even having the
of_overlay_mutex? Seems like we should just drop it.

I also don't think we really need to hold the mutex during post-apply
notifiers. It also seems like some steps could be moved outside the
mutex(es) like init_overlay_changeset().

Rob