Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 21 Jul 2020 15:50:35 -0700 (PDT)
Subject:     Re: [PATCH 5.7 233/244] RISC-V: Acquire mmap lock before invoking walk_page_range
In-Reply-To: <20200720191403.GB1529125@kroah.com>
CC:     Atish Patra <Atish.Patra@wdc.com>, naresh.kamboju@linaro.org,
        stable@vger.kernel.org, linux-kernel@vger.kernel.org,
        zong.li@sifive.com, lkft-triage@lists.linaro.org
From:   Palmer Dabbelt <palmerdabbelt@google.com>
To:     Greg KH <gregkh@linuxfoundation.org>, walken@google.com
Message-ID: <mhng-903745bf-c5df-4e70-ade8-c1e596265fc4@palmerdabbelt-glaptop1>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 20 Jul 2020 12:14:03 PDT (-0700), Greg KH wrote:
> On Mon, Jul 20, 2020 at 06:50:10PM +0000, Atish Patra wrote:
>> On Mon, 2020-07-20 at 23:11 +0530, Naresh Kamboju wrote:
>> > RISC-V build breaks on stable-rc 5.7 branch.
>> > build failed with gcc-8, gcc-9 and gcc-9.
>> >
>>
>> Sorry for the compilation issue.
>>
>> mmap_read_lock was intrdouced in the following commit.
>>
>> commit 9740ca4e95b4
>> Author: Michel Lespinasse <walken@google.com>
>> Date:   Mon Jun 8 21:33:14 2020 -0700
>>
>>     mmap locking API: initial implementation as rwsem wrappers
>>
>> The following two commits replaced the usage of mmap_sem rwsem calls
>> with mmap_lock.
>>
>> d8ed45c5dcd4 (mmap locking API: use coccinelle to convert mmap_sem
>> rwsem call sites)
>> 89154dd5313f (mmap locking API: convert mmap_sem call sites missed by
>> coccinelle)
>>
>> The first commit is not present in stale 5.7-y for obvious reasons.
>>
>> Do we need to send a separate patch only for stable branch with
>> mmap_sem ? I am not sure if that will cause a conflict again in future.
>
> I do not like taking odd backports, and would rather take the real patch
> that is upstream.

I guess I'm a bit new to stable backports so I'm not sure what's expected here.
The failing patch fixes a bug by using a new interface.  The smallest diff fix
for the stable kernels would be to construct a similar fix without the new
interface, which in this case is very easy as the new interface just converted
some generic locking calls to one-line functions.  It seems somewhat circuitous
to land that in Linus' tree, though, as it would require breaking our port
before fixing it to use the old interfaces and then cleaning it up to use the
new interfaces.

Are we expected to pull the new interface onto stable in addition to this fix?
The new interface doesn't actually fix anything itself, but it would allow a
functional kernel to be constructed that consisted of only backports from
Linus' tree (which would also make further fixes easier).  It seems safe to
just pull in 9740ca4e95b4 ("mmap locking API: initial implementation as rwsem
wrappers") before this failing patch, as in this case the new interface will
function correctly with only a subset of callers having been converted.  Of
course that's not a generally true statement so I don't know if future code
will behave that way, but pulling in those conversion patches is definitely
unnecessary diff right now.

> I will drop this patch from the tree now, so everyone can figure out
> what they want to do in the future :)

That certainly seems like the right way to go for now, thanks!