From: Atish Patra <[email protected]>
commit 0e2c09011d4de4161f615ff860a605a9186cf62a upstream.
As per walk_page_range documentation, mmap lock should be acquired by the
caller before invoking walk_page_range. mmap_assert_locked gets triggered
without that. The details can be found here.
http://lists.infradead.org/pipermail/linux-riscv/2020-June/010335.html
Fixes: 395a21ff859c(riscv: add ARCH_HAS_SET_DIRECT_MAP support)
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Michel Lespinasse <[email protected]>
Reviewed-by: Zong Li <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/riscv/mm/pageattr.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
--- a/arch/riscv/mm/pageattr.c
+++ b/arch/riscv/mm/pageattr.c
@@ -151,6 +151,7 @@ int set_memory_nx(unsigned long addr, in
int set_direct_map_invalid_noflush(struct page *page)
{
+ int ret;
unsigned long start = (unsigned long)page_address(page);
unsigned long end = start + PAGE_SIZE;
struct pageattr_masks masks = {
@@ -158,11 +159,16 @@ int set_direct_map_invalid_noflush(struc
.clear_mask = __pgprot(_PAGE_PRESENT)
};
- return walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
+ mmap_read_lock(&init_mm);
+ ret = walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
+ mmap_read_unlock(&init_mm);
+
+ return ret;
}
int set_direct_map_default_noflush(struct page *page)
{
+ int ret;
unsigned long start = (unsigned long)page_address(page);
unsigned long end = start + PAGE_SIZE;
struct pageattr_masks masks = {
@@ -170,7 +176,11 @@ int set_direct_map_default_noflush(struc
.clear_mask = __pgprot(0)
};
- return walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
+ mmap_read_lock(&init_mm);
+ ret = walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
+ mmap_read_unlock(&init_mm);
+
+ return ret;
}
void __kernel_map_pages(struct page *page, int numpages, int enable)
RISC-V build breaks on stable-rc 5.7 branch.
build failed with gcc-8, gcc-9 and gcc-9.
On Mon, 20 Jul 2020 at 21:46, Greg Kroah-Hartman
<[email protected]> wrote:
>
> From: Atish Patra <[email protected]>
>
> commit 0e2c09011d4de4161f615ff860a605a9186cf62a upstream.
>
> As per walk_page_range documentation, mmap lock should be acquired by the
> caller before invoking walk_page_range. mmap_assert_locked gets triggered
> without that. The details can be found here.
>
> http://lists.infradead.org/pipermail/linux-riscv/2020-June/010335.html
>
> Fixes: 395a21ff859c(riscv: add ARCH_HAS_SET_DIRECT_MAP support)
> Signed-off-by: Atish Patra <[email protected]>
> Reviewed-by: Michel Lespinasse <[email protected]>
> Reviewed-by: Zong Li <[email protected]>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> ---
> arch/riscv/mm/pageattr.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> --- a/arch/riscv/mm/pageattr.c
> +++ b/arch/riscv/mm/pageattr.c
> @@ -151,6 +151,7 @@ int set_memory_nx(unsigned long addr, in
>
> int set_direct_map_invalid_noflush(struct page *page)
> {
> + int ret;
> unsigned long start = (unsigned long)page_address(page);
> unsigned long end = start + PAGE_SIZE;
> struct pageattr_masks masks = {
> @@ -158,11 +159,16 @@ int set_direct_map_invalid_noflush(struc
> .clear_mask = __pgprot(_PAGE_PRESENT)
> };
>
> - return walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
> + mmap_read_lock(&init_mm);
> + ret = walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
> + mmap_read_unlock(&init_mm);
make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=riscv
CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
riscv64-linux-gnu-gcc" O=build defconfig
#
#
make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j32 ARCH=riscv
CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
riscv64-linux-gnu-gcc" O=build
#
../arch/riscv/mm/pageattr.c: In function ‘set_direct_map_invalid_noflush’:
../arch/riscv/mm/pageattr.c:162:2: error: implicit declaration of
function ‘mmap_read_lock’; did you mean ‘_raw_read_lock’?
[-Werror=implicit-function-declaration]
162 | mmap_read_lock(&init_mm);
| ^~~~~~~~~~~~~~
| _raw_read_lock
../arch/riscv/mm/pageattr.c:164:2: error: implicit declaration of
function ‘mmap_read_unlock’; did you mean ‘_raw_read_unlock’?
[-Werror=implicit-function-declaration]
164 | mmap_read_unlock(&init_mm);
| ^~~~~~~~~~~~~~~~
| _raw_read_unlock
cc1: some warnings being treated as errors
> +
> + return ret;
> }
>
> int set_direct_map_default_noflush(struct page *page)
> {
> + int ret;
> unsigned long start = (unsigned long)page_address(page);
> unsigned long end = start + PAGE_SIZE;
> struct pageattr_masks masks = {
> @@ -170,7 +176,11 @@ int set_direct_map_default_noflush(struc
> .clear_mask = __pgprot(0)
> };
>
> - return walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
> + mmap_read_lock(&init_mm);
> + ret = walk_page_range(&init_mm, start, end, &pageattr_ops, &masks);
> + mmap_read_unlock(&init_mm);
> +
> + return ret;
> }
>
> void __kernel_map_pages(struct page *page, int numpages, int enable)
>
ref:
full build log with default config.
https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/647154950
--
Linaro LKFT
https://lkft.linaro.org
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 <[email protected]>
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.
> On Mon, 20 Jul 2020 at 21:46, Greg Kroah-Hartman
> <[email protected]> wrote:
> > From: Atish Patra <[email protected]>
> >
> > commit 0e2c09011d4de4161f615ff860a605a9186cf62a upstream.
> >
> > As per walk_page_range documentation, mmap lock should be acquired
> > by the
> > caller before invoking walk_page_range. mmap_assert_locked gets
> > triggered
> > without that. The details can be found here.
> >
> > http://lists.infradead.org/pipermail/linux-riscv/2020-June/010335.html
> >
> > Fixes: 395a21ff859c(riscv: add ARCH_HAS_SET_DIRECT_MAP support)
> > Signed-off-by: Atish Patra <[email protected]>
> > Reviewed-by: Michel Lespinasse <[email protected]>
> > Reviewed-by: Zong Li <[email protected]>
> > Signed-off-by: Palmer Dabbelt <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
> > ---
> > arch/riscv/mm/pageattr.c | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > --- a/arch/riscv/mm/pageattr.c
> > +++ b/arch/riscv/mm/pageattr.c
> > @@ -151,6 +151,7 @@ int set_memory_nx(unsigned long addr, in
> >
> > int set_direct_map_invalid_noflush(struct page *page)
> > {
> > + int ret;
> > unsigned long start = (unsigned long)page_address(page);
> > unsigned long end = start + PAGE_SIZE;
> > struct pageattr_masks masks = {
> > @@ -158,11 +159,16 @@ int set_direct_map_invalid_noflush(struc
> > .clear_mask = __pgprot(_PAGE_PRESENT)
> > };
> >
> > - return walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > + mmap_read_lock(&init_mm);
> > + ret = walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > + mmap_read_unlock(&init_mm);
>
> make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=riscv
> CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
> riscv64-linux-gnu-gcc" O=build defconfig
> #
> #
> make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j32 ARCH=riscv
> CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
> riscv64-linux-gnu-gcc" O=build
> #
> ../arch/riscv/mm/pageattr.c: In function
> ‘set_direct_map_invalid_noflush’:
> ../arch/riscv/mm/pageattr.c:162:2: error: implicit declaration of
> function ‘mmap_read_lock’; did you mean ‘_raw_read_lock’?
> [-Werror=implicit-function-declaration]
> 162 | mmap_read_lock(&init_mm);
> | ^~~~~~~~~~~~~~
> | _raw_read_lock
> ../arch/riscv/mm/pageattr.c:164:2: error: implicit declaration of
> function ‘mmap_read_unlock’; did you mean ‘_raw_read_unlock’?
> [-Werror=implicit-function-declaration]
> 164 | mmap_read_unlock(&init_mm);
> | ^~~~~~~~~~~~~~~~
> | _raw_read_unlock
> cc1: some warnings being treated as errors
>
> > +
> > + return ret;
> > }
> >
> > int set_direct_map_default_noflush(struct page *page)
> > {
> > + int ret;
> > unsigned long start = (unsigned long)page_address(page);
> > unsigned long end = start + PAGE_SIZE;
> > struct pageattr_masks masks = {
> > @@ -170,7 +176,11 @@ int set_direct_map_default_noflush(struc
> > .clear_mask = __pgprot(0)
> > };
> >
> > - return walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > + mmap_read_lock(&init_mm);
> > + ret = walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > + mmap_read_unlock(&init_mm);
> > +
> > + return ret;
> > }
> >
> > void __kernel_map_pages(struct page *page, int numpages, int
> > enable)
> >
>
> ref:
> full build log with default config.
> https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/647154950
>
--
Regards,
Atish
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 <[email protected]>
> 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 will drop this patch from the tree now, so everyone can figure out
what they want to do in the future :)
thanks,
greg k-h
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 <[email protected]>
>> 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!
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 <[email protected]>
> > > 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 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
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 <[email protected]>
> > > > 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 can combine all the git ids that needs to be pulled in.
> > 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
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 <[email protected]>
> > > > > 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 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
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 <[email protected]>
> > > > > > 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