| Message ID | 20170311183016.GA20514@ls3530.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH] linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures
Message-id: 20170311183016.GA20514@ls3530.fritz.box
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20170311034232.14213-1-rth@twiddle.net -> patchew/20170311034232.14213-1-rth@twiddle.net
* [new tag] patchew/20170311183016.GA20514@ls3530.fritz.box -> patchew/20170311183016.GA20514@ls3530.fritz.box
Switched to a new branch 'test'
8c465a2 linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures
=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: Add TARGET_MAP_STACK and TARGET_MAP_HUGETLB for all remaining architectures...
ERROR: code indent should never use tabs
#23: FILE: linux-user/syscall.c:5878:
+^I{ TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK },$
ERROR: code indent should never use tabs
#24: FILE: linux-user/syscall.c:5879:
+^I{ TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB },$
ERROR: line over 90 characters
#36: FILE: linux-user/syscall_defs.h:1324:
+#define TARGET_MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
ERROR: code indent should never use tabs
#36: FILE: linux-user/syscall_defs.h:1324:
+#define TARGET_MAP_STACK^I0x40000^I^I/* give out an address that is best suited for process/thread stacks */$
ERROR: code indent should never use tabs
#37: FILE: linux-user/syscall_defs.h:1325:
+#define TARGET_MAP_HUGETLB^I0x80000^I^I/* create a huge page mapping */$
ERROR: line over 90 characters
#45: FILE: linux-user/syscall_defs.h:1336:
+#define TARGET_MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
ERROR: code indent should never use tabs
#45: FILE: linux-user/syscall_defs.h:1336:
+#define TARGET_MAP_STACK^I0x20000^I^I/* give out an address that is best suited for process/thread stacks */$
ERROR: code indent should never use tabs
#46: FILE: linux-user/syscall_defs.h:1337:
+#define TARGET_MAP_HUGETLB^I0x40000^I^I/* create a huge page mapping */$
ERROR: line over 90 characters
#54: FILE: linux-user/syscall_defs.h:1348:
+#define TARGET_MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
ERROR: code indent should never use tabs
#54: FILE: linux-user/syscall_defs.h:1348:
+#define TARGET_MAP_STACK^I0x80000^I^I/* give out an address that is best suited for process/thread stacks */$
ERROR: code indent should never use tabs
#55: FILE: linux-user/syscall_defs.h:1349:
+#define TARGET_MAP_HUGETLB^I0x100000^I/* create a huge page mapping */$
ERROR: line over 90 characters
#63: FILE: linux-user/syscall_defs.h:1370:
+#define TARGET_MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
ERROR: code indent should never use tabs
#63: FILE: linux-user/syscall_defs.h:1370:
+#define TARGET_MAP_STACK^I0x20000^I^I/* give out an address that is best suited for process/thread stacks */$
ERROR: code indent should never use tabs
#64: FILE: linux-user/syscall_defs.h:1371:
+#define TARGET_MAP_HUGETLB^I0x40000^I^I/* create a huge page mapping */$
total: 14 errors, 0 warnings, 40 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
On 03/12/2017 04:30 AM, Helge Deller wrote: > Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB > for alpha, mips, ppc and x86, and fix the mmap_flags translation table > to translate those flags between host and target architecture. > > Signed-off-by: Helge Deller <deller@gmx.de> > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index cec8428..03ed370 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -5875,6 +5875,8 @@ static bitmask_transtbl mmap_flags_tbl[] = { > { TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED }, > { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE, > MAP_NORESERVE }, > + { TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK }, > + { TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB }, I don't see any point in this. First, MAP_STACK is ignored by the kernel, and has been for some time. Second, the size of huge pages varies widely between different targets, and we're not really able to map the sizes between guest and host. I suppose we could pass it through and get lucky when the sizes do match. But if they don't, is it better to succeed with small tlb entries or fail with -EINVAL? r~
On 11.03.2017 22:53, Richard Henderson wrote: > On 03/12/2017 04:30 AM, Helge Deller wrote: >> Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB >> for alpha, mips, ppc and x86, and fix the mmap_flags translation table >> to translate those flags between host and target architecture. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index cec8428..03ed370 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -5875,6 +5875,8 @@ static bitmask_transtbl mmap_flags_tbl[] = { >> { TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED }, >> { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE, >> MAP_NORESERVE }, >> + { TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK }, >> + { TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB }, > > I don't see any point in this. First, MAP_STACK is ignored by the > kernel, and has been for some time. Yes, the kernel ignores the native MAP_STACK define. > Second, the size of huge pages varies widely between different targets, > and we're not really able to map the sizes between guest and host. This might probably not be necessary, if the userspace app simply asks which huge page sizes are supported (e.g. by procfs entries or via "hugeadm --page-sizes-all"). But the main intention of my patch was not to fix MAP_STACK or MAP_HUGETLB, but more to ensure that those flags are avoided to be handed over to the host. For example, TARGET_MAP_STACK is 0x40000 on hppa, and if it's not filtered out in e.g. a mmap() call, then this becomes to MAP_HUGETLB on x86_64 (which is 0x40000 on x86_64). Helge
On 03/14/2017 12:09 AM, Helge Deller wrote: > But the main intention of my patch was not to fix MAP_STACK or MAP_HUGETLB, > but more to ensure that those flags are avoided to be handed over to the host. > For example, TARGET_MAP_STACK is 0x40000 on hppa, and if it's not filtered out > in e.g. a mmap() call, then this becomes to MAP_HUGETLB on x86_64 (which is > 0x40000 on x86_64). Ah... yes. That is a much better point. r~
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index cec8428..03ed370 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5875,6 +5875,8 @@ static bitmask_transtbl mmap_flags_tbl[] = { { TARGET_MAP_LOCKED, TARGET_MAP_LOCKED, MAP_LOCKED, MAP_LOCKED }, { TARGET_MAP_NORESERVE, TARGET_MAP_NORESERVE, MAP_NORESERVE, MAP_NORESERVE }, + { TARGET_MAP_STACK, TARGET_MAP_STACK, MAP_STACK, MAP_STACK }, + { TARGET_MAP_HUGETLB, TARGET_MAP_HUGETLB, MAP_HUGETLB, MAP_HUGETLB }, { 0, 0, 0, 0 } }; diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index f356189..a516d77 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -1346,6 +1346,8 @@ struct target_winsize { #define TARGET_MAP_NORESERVE 0x0400 /* don't check for reservations */ #define TARGET_MAP_POPULATE 0x10000 /* populate (prefault) pagetables */ #define TARGET_MAP_NONBLOCK 0x20000 /* do not block on IO */ +#define TARGET_MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */ +#define TARGET_MAP_HUGETLB 0x80000 /* create a huge page mapping */ #elif defined(TARGET_PPC) #define TARGET_MAP_FIXED 0x10 /* Interpret addr exactly */ #define TARGET_MAP_ANONYMOUS 0x20 /* don't use a file */ @@ -1356,6 +1358,8 @@ struct target_winsize { #define TARGET_MAP_NORESERVE 0x0040 /* don't check for reservations */ #define TARGET_MAP_POPULATE 0x8000 /* populate (prefault) pagetables */ #define TARGET_MAP_NONBLOCK 0x10000 /* do not block on IO */ +#define TARGET_MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */ +#define TARGET_MAP_HUGETLB 0x40000 /* create a huge page mapping */ #elif defined(TARGET_ALPHA) #define TARGET_MAP_ANONYMOUS 0x10 /* don't use a file */ #define TARGET_MAP_FIXED 0x100 /* Interpret addr exactly */ @@ -1366,6 +1370,8 @@ struct target_winsize { #define TARGET_MAP_NORESERVE 0x10000 /* no check for reservations */ #define TARGET_MAP_POPULATE 0x20000 /* pop (prefault) pagetables */ #define TARGET_MAP_NONBLOCK 0x40000 /* do not block on IO */ +#define TARGET_MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */ +#define TARGET_MAP_HUGETLB 0x100000 /* create a huge page mapping */ #elif defined(TARGET_HPPA) #define TARGET_MAP_ANONYMOUS 0x10 /* don't use a file */ #define TARGET_MAP_FIXED 0x04 /* Interpret addr exactly */ @@ -1388,6 +1394,8 @@ struct target_winsize { #define TARGET_MAP_NORESERVE 0x4000 /* don't check for reservations */ #define TARGET_MAP_POPULATE 0x8000 /* populate (prefault) pagetables */ #define TARGET_MAP_NONBLOCK 0x10000 /* do not block on IO */ +#define TARGET_MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */ +#define TARGET_MAP_HUGETLB 0x40000 /* create a huge page mapping */ #define TARGET_MAP_UNINITIALIZED 0x4000000 /* for anonymous mmap, memory could be uninitialized */ #endif
Add the missing defines and for TARGET_MAP_STACK and TARGET_MAP_HUGETLB for alpha, mips, ppc and x86, and fix the mmap_flags translation table to translate those flags between host and target architecture. Signed-off-by: Helge Deller <deller@gmx.de>