| Message ID | 1627475340-128057-1-git-send-email-baolin.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | selftests: openat2: Fix testing failure for O_LARGEFILE flag | expand |
Hi, > When running the openat2 test suite on ARM64 platform, we got below failure, > since the definition of the O_LARGEFILE is different on ARM64. So we can > set the correct O_LARGEFILE definition on ARM64 to fix this issue. Sorry, I forgot to copy the failure log: # openat2 unexpectedly returned # 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] with 208000 (!= 208000) not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails with -22 (Invalid argument) > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > --- > tools/testing/selftests/openat2/openat2_test.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c > index d7ec1e7..1bddbe9 100644 > --- a/tools/testing/selftests/openat2/openat2_test.c > +++ b/tools/testing/selftests/openat2/openat2_test.c > @@ -22,7 +22,11 @@ > * XXX: This is wrong on {mips, parisc, powerpc, sparc}. > */ > #undef O_LARGEFILE > +#ifdef __aarch64__ > +#define O_LARGEFILE 0x20000 > +#else > #define O_LARGEFILE 0x8000 > +#endif > > struct open_how_ext { > struct open_how inner; >
Hi Shuah, On 2021/7/28 20:32, Baolin Wang wrote: > Hi, > >> When running the openat2 test suite on ARM64 platform, we got below >> failure, >> since the definition of the O_LARGEFILE is different on ARM64. So we can >> set the correct O_LARGEFILE definition on ARM64 to fix this issue. > > Sorry, I forgot to copy the failure log: > > # openat2 unexpectedly returned # > 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] > with 208000 (!= 208000) > not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails > with -22 (Invalid argument) > >> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> Could you apply this patch if no objection from your side? Thanks. >> --- >> tools/testing/selftests/openat2/openat2_test.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/tools/testing/selftests/openat2/openat2_test.c >> b/tools/testing/selftests/openat2/openat2_test.c >> index d7ec1e7..1bddbe9 100644 >> --- a/tools/testing/selftests/openat2/openat2_test.c >> +++ b/tools/testing/selftests/openat2/openat2_test.c >> @@ -22,7 +22,11 @@ >> * XXX: This is wrong on {mips, parisc, powerpc, sparc}. >> */ >> #undef O_LARGEFILE >> +#ifdef __aarch64__ >> +#define O_LARGEFILE 0x20000 >> +#else >> #define O_LARGEFILE 0x8000 >> +#endif >> struct open_how_ext { >> struct open_how inner; >>
Hi Baolin, On 8/22/21 8:40 PM, Baolin Wang wrote: > Hi Shuah, > > On 2021/7/28 20:32, Baolin Wang wrote: >> Hi, >> >>> When running the openat2 test suite on ARM64 platform, we got below failure, >>> since the definition of the O_LARGEFILE is different on ARM64. So we can >>> set the correct O_LARGEFILE definition on ARM64 to fix this issue. >> >> Sorry, I forgot to copy the failure log: >> Please cc everybody get_maintainers.pl suggests. You are missing key reviewers for this change. Adding Christian Brauner and Aleksa Sarai to the thread. >> # openat2 unexpectedly returned # 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] with 208000 (!= 208000) Not sure I understand this. 208000 (!= 208000) look sthe same to me. >> not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails with -22 (Invalid argument) >> >>> >>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > > Could you apply this patch if no objection from your side? Thanks. > Ideally this define should come from an include file. Christian, Aleksa, Can you review this patch and let me know if this approach looks right. >>> --- >>> tools/testing/selftests/openat2/openat2_test.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c >>> index d7ec1e7..1bddbe9 100644 >>> --- a/tools/testing/selftests/openat2/openat2_test.c >>> +++ b/tools/testing/selftests/openat2/openat2_test.c >>> @@ -22,7 +22,11 @@ >>> * XXX: This is wrong on {mips, parisc, powerpc, sparc}. >>> */ >>> #undef O_LARGEFILE >>> +#ifdef __aarch64__ >>> +#define O_LARGEFILE 0x20000 >>> +#else >>> #define O_LARGEFILE 0x8000 >>> +#endif >>> struct open_how_ext { >>> struct open_how inner; >>> > thanks, -- Shuah
Hi Shuah, On 2021/8/24 3:23, Shuah Khan wrote: > Hi Baolin, > > On 8/22/21 8:40 PM, Baolin Wang wrote: >> Hi Shuah, >> >> On 2021/7/28 20:32, Baolin Wang wrote: >>> Hi, >>> >>>> When running the openat2 test suite on ARM64 platform, we got below >>>> failure, >>>> since the definition of the O_LARGEFILE is different on ARM64. So we >>>> can >>>> set the correct O_LARGEFILE definition on ARM64 to fix this issue. >>> >>> Sorry, I forgot to copy the failure log: >>> > > Please cc everybody get_maintainers.pl suggests. You are missing > key reviewers for this change. > > Adding Christian Brauner and Aleksa Sarai to the thread. Thanks. > >>> # openat2 unexpectedly returned # >>> 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] >>> with 208000 (!= 208000) > > Not sure I understand this. 208000 (!= 208000) look sthe same to me. These are not the error message, just show the fd flags. The error is it should return -22 (-EINVAL) for this test case, but it returns 3 which indicates a successful openat2() calling. >>> not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) >>> fails with -22 (Invalid argument) >>> >>>> >>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> >> Could you apply this patch if no objection from your side? Thanks. >> > > Ideally this define should come from an include file. > > Christian, Aleksa, > > Can you review this patch and let me know if this approach looks right. > >>>> --- >>>> tools/testing/selftests/openat2/openat2_test.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/tools/testing/selftests/openat2/openat2_test.c >>>> b/tools/testing/selftests/openat2/openat2_test.c >>>> index d7ec1e7..1bddbe9 100644 >>>> --- a/tools/testing/selftests/openat2/openat2_test.c >>>> +++ b/tools/testing/selftests/openat2/openat2_test.c >>>> @@ -22,7 +22,11 @@ >>>> * XXX: This is wrong on {mips, parisc, powerpc, sparc}. >>>> */ >>>> #undef O_LARGEFILE >>>> +#ifdef __aarch64__ >>>> +#define O_LARGEFILE 0x20000 >>>> +#else >>>> #define O_LARGEFILE 0x8000 >>>> +#endif >>>> struct open_how_ext { >>>> struct open_how inner; >>>> >> > > thanks, > -- Shuah
On 2021-08-23, Shuah Khan <skhan@linuxfoundation.org> wrote: > Hi Baolin, > > On 8/22/21 8:40 PM, Baolin Wang wrote: > > Hi Shuah, > > > > On 2021/7/28 20:32, Baolin Wang wrote: > > > Hi, > > > > > > > When running the openat2 test suite on ARM64 platform, we got below failure, > > > > since the definition of the O_LARGEFILE is different on ARM64. So we can > > > > set the correct O_LARGEFILE definition on ARM64 to fix this issue. > > > > > > Sorry, I forgot to copy the failure log: > > > > > Please cc everybody get_maintainers.pl suggests. You are missing > key reviewers for this change. > > Adding Christian Brauner and Aleksa Sarai to the thread. > > > > # openat2 unexpectedly returned # 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] with 208000 (!= 208000) > > Not sure I understand this. 208000 (!= 208000) look sthe same to me. > > > > not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails with -22 (Invalid argument) > > > > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > > > > Could you apply this patch if no objection from your side? Thanks. > > > > Ideally this define should come from an include file. The issue is that O_LARGEFILE is set to 0 by glibc because glibc appears to hide the nuts and bolts of largefile support from userspace. I couldn't find a nice way of doing a architecture-dependent includes of include/uapi from kselftests, so I just went with this instead -- but I agree that a proper include would be better if someone can figure out how to do it. > Christian, Aleksa, > > Can you review this patch and let me know if this approach looks right. Reviewed-by: Aleksa Sarai <cyphar@cyphar.com> It'd be nice to fix this for the other architectures I mention in the comment (mips, parisc, powerpc, sparc) -- which are the ones that I could find that had a custom O_LARGEFILE definition. > > > > --- > > > > tools/testing/selftests/openat2/openat2_test.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c > > > > index d7ec1e7..1bddbe9 100644 > > > > --- a/tools/testing/selftests/openat2/openat2_test.c > > > > +++ b/tools/testing/selftests/openat2/openat2_test.c > > > > @@ -22,7 +22,11 @@ > > > > * XXX: This is wrong on {mips, parisc, powerpc, sparc}. > > > > */ > > > > #undef O_LARGEFILE > > > > +#ifdef __aarch64__ > > > > +#define O_LARGEFILE 0x20000 > > > > +#else > > > > #define O_LARGEFILE 0x8000 > > > > +#endif > > > > struct open_how_ext { > > > > struct open_how inner; > > > > > > > > thanks, > -- Shuah
On Tue, Aug 24, 2021 at 09:21:29PM +1000, Aleksa Sarai wrote: > On 2021-08-23, Shuah Khan <skhan@linuxfoundation.org> wrote: > > Hi Baolin, > > > > On 8/22/21 8:40 PM, Baolin Wang wrote: > > > Hi Shuah, > > > > > > On 2021/7/28 20:32, Baolin Wang wrote: > > > > Hi, > > > > > > > > > When running the openat2 test suite on ARM64 platform, we got below failure, > > > > > since the definition of the O_LARGEFILE is different on ARM64. So we can > > > > > set the correct O_LARGEFILE definition on ARM64 to fix this issue. > > > > > > > > Sorry, I forgot to copy the failure log: > > > > > > > > Please cc everybody get_maintainers.pl suggests. You are missing > > key reviewers for this change. > > > > Adding Christian Brauner and Aleksa Sarai to the thread. > > > > > > # openat2 unexpectedly returned # 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] with 208000 (!= 208000) > > > > Not sure I understand this. 208000 (!= 208000) look sthe same to me. > > > > > > not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails with -22 (Invalid argument) > > > > > > > > > > > > > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> > > > > > > Could you apply this patch if no objection from your side? Thanks. > > > > > > > Ideally this define should come from an include file. > > The issue is that O_LARGEFILE is set to 0 by glibc because glibc appears > to hide the nuts and bolts of largefile support from userspace. I > couldn't find a nice way of doing a architecture-dependent includes of > include/uapi from kselftests, so I just went with this instead -- but I > agree that a proper include would be better if someone can figure out > how to do it. I'd just add arch-dependent defines for now and call it good. So seems good enough for me: Thanks! Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > > > Christian, Aleksa, > > > > Can you review this patch and let me know if this approach looks right. > > Reviewed-by: Aleksa Sarai <cyphar@cyphar.com>
On 8/24/21 5:36 AM, Christian Brauner wrote: > On Tue, Aug 24, 2021 at 09:21:29PM +1000, Aleksa Sarai wrote: >> On 2021-08-23, Shuah Khan <skhan@linuxfoundation.org> wrote: >>> Hi Baolin, >>> >>> On 8/22/21 8:40 PM, Baolin Wang wrote: >>>> Hi Shuah, >>>> >>>> On 2021/7/28 20:32, Baolin Wang wrote: >>>>> Hi, >>>>> >>>>>> When running the openat2 test suite on ARM64 platform, we got below failure, >>>>>> since the definition of the O_LARGEFILE is different on ARM64. So we can >>>>>> set the correct O_LARGEFILE definition on ARM64 to fix this issue. >>>>> >>>>> Sorry, I forgot to copy the failure log: >>>>> >>> >>> Please cc everybody get_maintainers.pl suggests. You are missing >>> key reviewers for this change. >>> >>> Adding Christian Brauner and Aleksa Sarai to the thread. >>> >>>>> # openat2 unexpectedly returned # 3['/lkp/benchmarks/kernel_selftests/tools/testing/selftests/openat2'] with 208000 (!= 208000) >>> >>> Not sure I understand this. 208000 (!= 208000) look sthe same to me. >>> >>>>> not ok 102 openat2 with incompatible flags (O_PATH | O_LARGEFILE) fails with -22 (Invalid argument) >>>>> >>>>>> >>>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >>>> >>>> Could you apply this patch if no objection from your side? Thanks. >>>> >>> >>> Ideally this define should come from an include file. >> >> The issue is that O_LARGEFILE is set to 0 by glibc because glibc appears >> to hide the nuts and bolts of largefile support from userspace. I >> couldn't find a nice way of doing a architecture-dependent includes of >> include/uapi from kselftests, so I just went with this instead -- but I >> agree that a proper include would be better if someone can figure out >> how to do it. > From a quick look, it will take sone work to consolidate multiple O_LARGEFILE defines. > I'd just add arch-dependent defines for now and call it good. So seems > good enough for me: > > Thanks! > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > >> >>> Christian, Aleksa, >>> >>> Can you review this patch and let me know if this approach looks right. >> >> Reviewed-by: Aleksa Sarai <cyphar@cyphar.com> Thank you for the patch and the reviews. I will apply this for 5.15-rc1 thanks, -- Shuah
On 8/24/21 8:33 AM, Shuah Khan wrote: > On 8/24/21 5:36 AM, Christian Brauner wrote: >> On Tue, Aug 24, 2021 at 09:21:29PM +1000, Aleksa Sarai wrote: >>> On 2021-08-23, Shuah Khan <skhan@linuxfoundation.org> wrote: >>>> Hi Baolin, >>>> >>>> On 8/22/21 8:40 PM, Baolin Wang wrote: >>>>> Hi Shuah, >>>>> >>>>> On 2021/7/28 20:32, Baolin Wang wrote: >>>>>> Hi, >>>>>> >>>>>>> When running the openat2 test suite on ARM64 platform, we got below failure, >>>>>>> since the definition of the O_LARGEFILE is different on ARM64. So we can >>>>>>> set the correct O_LARGEFILE definition on ARM64 to fix this issue. >>>>>> >>>>>> Sorry, I forgot to copy the failure log: >>>>>> Please send me v2 with failure log included in the commit log. thanks, -- Shuah
On 2021/8/25 0:50, Shuah Khan wrote: > On 8/24/21 8:33 AM, Shuah Khan wrote: >> On 8/24/21 5:36 AM, Christian Brauner wrote: >>> On Tue, Aug 24, 2021 at 09:21:29PM +1000, Aleksa Sarai wrote: >>>> On 2021-08-23, Shuah Khan <skhan@linuxfoundation.org> wrote: >>>>> Hi Baolin, >>>>> >>>>> On 8/22/21 8:40 PM, Baolin Wang wrote: >>>>>> Hi Shuah, >>>>>> >>>>>> On 2021/7/28 20:32, Baolin Wang wrote: >>>>>>> Hi, >>>>>>> >>>>>>>> When running the openat2 test suite on ARM64 platform, we got >>>>>>>> below failure, >>>>>>>> since the definition of the O_LARGEFILE is different on ARM64. >>>>>>>> So we can >>>>>>>> set the correct O_LARGEFILE definition on ARM64 to fix this issue. >>>>>>> >>>>>>> Sorry, I forgot to copy the failure log: >>>>>>> > > Please send me v2 with failure log included in the commit log. Sure. Thanks for reviewing.
diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c index d7ec1e7..1bddbe9 100644 --- a/tools/testing/selftests/openat2/openat2_test.c +++ b/tools/testing/selftests/openat2/openat2_test.c @@ -22,7 +22,11 @@ * XXX: This is wrong on {mips, parisc, powerpc, sparc}. */ #undef O_LARGEFILE +#ifdef __aarch64__ +#define O_LARGEFILE 0x20000 +#else #define O_LARGEFILE 0x8000 +#endif struct open_how_ext { struct open_how inner;
When running the openat2 test suite on ARM64 platform, we got below failure, since the definition of the O_LARGEFILE is different on ARM64. So we can set the correct O_LARGEFILE definition on ARM64 to fix this issue. Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> --- tools/testing/selftests/openat2/openat2_test.c | 4 ++++ 1 file changed, 4 insertions(+)