lkml.org 
[lkml]   [2020]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 5.7 233/244] RISC-V: Acquire mmap lock before invoking walk_page_range
    Date
    On Thu, 2020-07-23 at 13:46 +0900, Stafford Horne wrote:
    > On Wed, Jul 22, 2020 at 03:11:35PM +0000, Atish Patra wrote:
    > > On Wed, 2020-07-22 at 14:48 +0200, Greg KH wrote:
    > > > On Tue, Jul 21, 2020 at 03:50:35PM -0700, Palmer Dabbelt wrote:
    > > > > 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?
    > > >
    > > > If it really does fix a big issue, yes, that is fine to do.
    > > >
    > >
    > > The original issue was manifests only for certain rootfs with
    > > CONFIG_DEBUG_VM enabled in kernel. I am not sure if that qualfies
    > > for a
    > > big issue :). But there was a similar fix for OpenRISC as well.
    > >
    > > +stafford (who fixed the issue for OpenRISC)
    > >
    > > @stafford Was there a request to backport the fix to stable ?
    >
    > I have not requested pulling my patch to stable. Mine is this one:
    >
    > 313a5257b84c2 ("openrisc: fix boot oops when DEBUG_VM is enabled")
    >
    > If you cat request that would be great.
    >
    > > I can combine all the git ids that needs to be pulled in.
    >
    > Note, mine lists:
    >
    > Fixes: 42fc541404f2 ("mmap locking API: add mmap_assert_locked()
    > and mmap_assert_write_locked()")
    >
    > while your's lists:
    >
    > Fixes: 395a21ff859c(riscv: add ARCH_HAS_SET_DIRECT_MAP support)
    >
    > That is when the code was introduced to the riscv port, but not the
    > commit that
    > broke booting.
    >
    > I think if you list the Fixes as I did when backporting to stable
    > Greg, or his
    > tools, would also know that the patch depends on the the 42fc541404f2
    > commit.
    >
    > Also, I guess there is no problem with listing 2 "Fixes" in the
    > future.
    >

    Thanks. I will keep that in mind in future.

    Backporting the RISC-V fix would require the original commit to be
    backported as well.

    commit 9740ca4e95b4 (mmap locking API: initial implementation as rwsem
    wrappers)

    However, that is the first commit for big cleanup 12 patch series.
    https://lkml.org/lkml/2020/5/20/56

    While backporting the first commit (9740ca4e95b4) would solve the
    problem for RISC-V, all other architecture fixes won't be there.
    Do we want that in stable tree?


    > Thanks Atish and Palmer.
    >
    > -Stafford
    >
    > > > > 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).
    > > >
    > > > That's fine.
    > > >
    > > > > 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.
    > > >
    > > > If someone wants to send me a full set of the git ids that need
    > > > to be
    > > > pulled in, I will be glad to do so.
    > > >
    > > > thanks,
    > > >
    > > > greg k-h
    > >
    > > --
    > > Regards,
    > > Atish

    --
    Regards,
    Atish
    \
     
     \ /
      Last update: 2020-07-27 19:31    [W:2.651 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site