| Message ID | 87lhmobvuu.fsf@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Dec 03, 2014 at 08:53:37PM +0530, Aneesh Kumar K.V wrote: > Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > > > On Tue, 2014-12-02 at 12:57 +0530, Aneesh Kumar K.V wrote: > >> Now, hash_preload can possibly insert an hpte in hash page table even if > >> the access is not allowed by the pte permissions. But i guess even that > >> is ok. because we will fault again, end-up calling hash_page_mm where we > >> handle that part correctly. > > > > I think we need a test case... > > > > I ran the subpageprot test that Paul had written. I modified it to ran > with selftest. > It's implied but can I assume it passed? If so, Ben and Paul, can I consider the series to be acked by you other than the minor comment updates? Thanks.
Mel Gorman <mgorman@suse.de> writes: > On Wed, Dec 03, 2014 at 08:53:37PM +0530, Aneesh Kumar K.V wrote: >> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: >> >> > On Tue, 2014-12-02 at 12:57 +0530, Aneesh Kumar K.V wrote: >> >> Now, hash_preload can possibly insert an hpte in hash page table even if >> >> the access is not allowed by the pte permissions. But i guess even that >> >> is ok. because we will fault again, end-up calling hash_page_mm where we >> >> handle that part correctly. >> > >> > I think we need a test case... >> > >> >> I ran the subpageprot test that Paul had written. I modified it to ran >> with selftest. >> > > It's implied but can I assume it passed? Yes. -bash-4.2# ./subpage_prot test: subpage_prot tags: git_version:v3.17-rc3-13511-g0cd3756 allocated malloc block of 0x4000000 bytes at 0x0x3fffb0d10000 testing malloc block... OK success: subpage_prot -bash-4.2#
On Wed, 2014-12-03 at 15:52 +0000, Mel Gorman wrote: > > It's implied but can I assume it passed? If so, Ben and Paul, can I > consider the series to be acked by you other than the minor comment > updates? Yes. Assuming it passed :-) Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cheers, Ben.
On Thu, Dec 04, 2014 at 08:01:57AM +1100, Benjamin Herrenschmidt wrote: > On Wed, 2014-12-03 at 15:52 +0000, Mel Gorman wrote: > > > > It's implied but can I assume it passed? If so, Ben and Paul, can I > > consider the series to be acked by you other than the minor comment > > updates? > > Yes. Assuming it passed :-) > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Sweet, thanks.
On Wed, Dec 03, 2014 at 10:50:35PM +0530, Aneesh Kumar K.V wrote: > Mel Gorman <mgorman@suse.de> writes: > > > On Wed, Dec 03, 2014 at 08:53:37PM +0530, Aneesh Kumar K.V wrote: > >> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > >> > >> > On Tue, 2014-12-02 at 12:57 +0530, Aneesh Kumar K.V wrote: > >> >> Now, hash_preload can possibly insert an hpte in hash page table even if > >> >> the access is not allowed by the pte permissions. But i guess even that > >> >> is ok. because we will fault again, end-up calling hash_page_mm where we > >> >> handle that part correctly. > >> > > >> > I think we need a test case... > >> > > >> > >> I ran the subpageprot test that Paul had written. I modified it to ran > >> with selftest. > >> > > > > It's implied but can I assume it passed? > > Yes. > > -bash-4.2# ./subpage_prot > test: subpage_prot > tags: git_version:v3.17-rc3-13511-g0cd3756 > allocated malloc block of 0x4000000 bytes at 0x0x3fffb0d10000 > testing malloc block... > OK > success: subpage_prot > -bash-4.2# > Thanks for adding that and double checking. I won't pick up the patch as part of this series because it's not directly related but I would strongly suggest sending the patch separately.
diff --git a/tools/testing/selftests/powerpc/mm/Makefile b/tools/testing/selftests/powerpc/mm/Makefile index 357ccbd6bad9..fb00c6f7d675 100644 --- a/tools/testing/selftests/powerpc/mm/Makefile +++ b/tools/testing/selftests/powerpc/mm/Makefile @@ -1,7 +1,7 @@ noarg: $(MAKE) -C ../ -PROGS := hugetlb_vs_thp_test +PROGS := hugetlb_vs_thp_test subpage_prot all: $(PROGS) diff --git a/tools/testing/selftests/powerpc/mm/subpage_prot.c b/tools/testing/selftests/powerpc/mm/subpage_prot.c new file mode 100644 index 000000000000..1efeafc2e175 --- /dev/null +++ b/tools/testing/selftests/powerpc/mm/subpage_prot.c @@ -0,0 +1,150 @@ +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <signal.h> +#include <stdarg.h> +#include <sys/ptrace.h> +#include <sys/mman.h> +#include <errno.h> +#include <ucontext.h> +#include <assert.h> + +#include "utils.h" + +void *mallocblock; +unsigned long mallocsize; +void *fileblock; +off_t filesize; + +int in_test; +volatile int faulted; +volatile void *dar; +int errors; + +static void segv(int signum, siginfo_t *info, void *ctxt_v) +{ + ucontext_t *ctxt = (ucontext_t *)ctxt_v; + struct pt_regs *regs = ctxt->uc_mcontext.regs; + + if (!in_test) { + fprintf(stderr, "Segfault outside of test !\n"); + exit(1); + } + faulted = 1; + dar = (void *)regs->dar; + regs->nip += 4; +} + +static inline void do_read(const volatile void *addr) +{ + int ret; + asm volatile("lwz %0,0(%1); twi 0,%0,0; isync;\n" + : "=r" (ret) : "r" (addr) : "memory"); +} + +static inline void do_write(const volatile void *addr) +{ + int val = 0x1234567; + asm volatile("stw %0,0(%1); sync; \n" + : : "r" (val), "r" (addr) : "memory"); +} + +static inline void check_faulted(void *addr, long page, long subpage, int write) +{ + int want_fault = (subpage == ((page + 3) % 16)); + + if (write) + want_fault |= (subpage == ((page + 1) % 16)); + + if (faulted != want_fault) { + printf("Failed at 0x%p (p=%ld,sp=%ld,w=%d), want=%s, got=%s !\n", + addr, page, subpage, write, + want_fault ? "fault" : "pass", + faulted ? "fault" : "pass"); + ++errors; + } + if (faulted) { + if (dar != addr) { + printf("Fault expected at 0x%p and happened at 0x%p !\n", + addr, dar); + } + faulted = 0; + asm volatile("sync" : : : "memory"); + } +} + +static int run_test(void *addr, unsigned long size) +{ + unsigned int *map; + long i, j, pages, err; + + pages = size / 0x10000; + map = malloc(pages * 4); + assert(map); + + /* for each page, mark subpage i % 16 read only and subpage + * (i + 3) % 16 inaccessible + */ + for (i = 0; i < pages; i++) + map[i] = (0x40000000 >> (((i + 1) * 2) % 32)) | + (0xc0000000 >> (((i + 3) * 2) % 32)); + err = syscall(310, addr, size, map); + if (err) { + perror("subpage_perm"); + return 1; + } + free(map); + + in_test = 1; + errors = 0; + for (i = 0; i < pages; i++) + for (j = 0; j < 16; j++, addr += 0x1000) { + do_read(addr); + check_faulted(addr, i, j, 0); + do_write(addr); + check_faulted(addr, i, j, 1); + } + in_test = 0; + if (errors) { + printf("%d errors detected\n", errors); + return 1; + } + printf("OK\n"); + return 0; +} + +int test_main(void) +{ + unsigned long align; + + if (getpagesize() != 0x10000) { + fprintf(stderr, "Kernel page size must be 64K!\n"); + return 1; + } + + struct sigaction act = { + .sa_sigaction = segv, + .sa_flags = SA_SIGINFO + }; + sigaction(SIGSEGV, &act, NULL); + + mallocsize = 4*16*1024*1024; + posix_memalign(&mallocblock, 64*1024, mallocsize); + assert(mallocblock); + align = (unsigned long)mallocblock; + if (align & 0xffff) + align = (align | 0xffff) + 1; + mallocblock = (void *)align; + + printf("allocated malloc block of 0x%lx bytes at 0x%p\n", + mallocsize, mallocblock); + + printf("testing malloc block...\n"); + return run_test(mallocblock, mallocsize); +} + +int main(void) +{ + return test_harness(test_main, "subpage_prot"); +}