| Message ID | 20221018152527.4636-2-rpalethorpe@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] ptrace07: Fix compilation when not on x86 | expand |
Hi Richard, On 2022-10-18 at 16:25:27 +0100, Richard Palethorpe wrote: > Use memalign instead because we live in the past. > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > Cc: Pengfei Xu <pengfei.xu@intel.com> > --- > testcases/kernel/syscalls/ptrace/ptrace07.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c b/testcases/kernel/syscalls/ptrace/ptrace07.c > index 6bd2b1062..a60c2a49e 100644 > --- a/testcases/kernel/syscalls/ptrace/ptrace07.c > +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c > @@ -35,6 +35,7 @@ > #include "config.h" > #include "ptrace.h" > #include "tst_test.h" > +#include "tst_safe_macros.h" > #include "lapi/cpuid.h" > > #ifndef PTRACE_GETREGSET > @@ -95,7 +96,7 @@ static void do_test(void) > * of the XSAVE/XRSTOR save area) required by enabled features in XCR0. > */ > __cpuid_count(CPUID_LEAF_XSTATE, ecx, eax, ebx, ecx, edx); > - xstate = aligned_alloc(64, ebx); > + xstate = SAFE_MEMALIGN(64, ebx); > struct iovec iov = { .iov_base = xstate, .iov_len = ebx }; > int status; > bool okay; Yes, it's better for LTP compilation. Thanks for the patch! I checked SAFE_MEMALIGN(), it will verify that the buffer is NULL or not. " rval = memalign(alignment, size); if (rval == NULL) { tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, "memalign() failed"); } " So could you remove below unnecessary lines in ptrace07.c, thanks. " - if (!xstate) - tst_brk(TBROK, "aligned_alloc() failed for xstate buffer"); " Thanks! BR. > -- > 2.36.1 >
On Wed, Oct 19, 2022 at 9:40 AM Pengfei Xu <pengfei.xu@intel.com> wrote: > Hi Richard, > > On 2022-10-18 at 16:25:27 +0100, Richard Palethorpe wrote: > > Use memalign instead because we live in the past. > > > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > Cc: Pengfei Xu <pengfei.xu@intel.com> > > --- > > testcases/kernel/syscalls/ptrace/ptrace07.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c > b/testcases/kernel/syscalls/ptrace/ptrace07.c > > index 6bd2b1062..a60c2a49e 100644 > > --- a/testcases/kernel/syscalls/ptrace/ptrace07.c > > +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c > > @@ -35,6 +35,7 @@ > > #include "config.h" > > #include "ptrace.h" > > #include "tst_test.h" > > +#include "tst_safe_macros.h" > > #include "lapi/cpuid.h" > > > > #ifndef PTRACE_GETREGSET > > @@ -95,7 +96,7 @@ static void do_test(void) > > * of the XSAVE/XRSTOR save area) required by enabled features in > XCR0. > > */ > > __cpuid_count(CPUID_LEAF_XSTATE, ecx, eax, ebx, ecx, edx); > > - xstate = aligned_alloc(64, ebx); > > + xstate = SAFE_MEMALIGN(64, ebx); > > struct iovec iov = { .iov_base = xstate, .iov_len = ebx }; > > int status; > > bool okay; > > Yes, it's better for LTP compilation. Thanks for the patch! > > I checked SAFE_MEMALIGN(), it will verify that the buffer is NULL or not. > " > rval = memalign(alignment, size); > > if (rval == NULL) { > tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, > "memalign() failed"); > } > " > So could you remove below unnecessary lines in ptrace07.c, thanks. > I helped modify this and pushed it, thanks!
Hi Li Wang, On 2022-10-19 at 10:59:54 +0800, Li Wang wrote: > On Wed, Oct 19, 2022 at 9:40 AM Pengfei Xu <pengfei.xu@intel.com> wrote: > > > Hi Richard, > > > > On 2022-10-18 at 16:25:27 +0100, Richard Palethorpe wrote: > > > Use memalign instead because we live in the past. > > > > > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > > Cc: Pengfei Xu <pengfei.xu@intel.com> > > > --- > > > testcases/kernel/syscalls/ptrace/ptrace07.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c > > b/testcases/kernel/syscalls/ptrace/ptrace07.c > > > index 6bd2b1062..a60c2a49e 100644 > > > --- a/testcases/kernel/syscalls/ptrace/ptrace07.c > > > +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c > > > @@ -35,6 +35,7 @@ > > > #include "config.h" > > > #include "ptrace.h" > > > #include "tst_test.h" > > > +#include "tst_safe_macros.h" > > > #include "lapi/cpuid.h" > > > > > > #ifndef PTRACE_GETREGSET > > > @@ -95,7 +96,7 @@ static void do_test(void) > > > * of the XSAVE/XRSTOR save area) required by enabled features in > > XCR0. > > > */ > > > __cpuid_count(CPUID_LEAF_XSTATE, ecx, eax, ebx, ecx, edx); > > > - xstate = aligned_alloc(64, ebx); > > > + xstate = SAFE_MEMALIGN(64, ebx); > > > struct iovec iov = { .iov_base = xstate, .iov_len = ebx }; > > > int status; > > > bool okay; > > > > Yes, it's better for LTP compilation. Thanks for the patch! > > > > I checked SAFE_MEMALIGN(), it will verify that the buffer is NULL or not. > > " > > rval = memalign(alignment, size); > > > > if (rval == NULL) { > > tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, > > "memalign() failed"); > > } > > " > > So could you remove below unnecessary lines in ptrace07.c, thanks. > > > > I helped modify this and pushed it, thanks! Great, thanks! I tried that "SAFE_MEMALIGN(64, ebx);" modification in ptrace07. Ptrace07 works well and passed in previous reproduced server. Thanks! BR. > > -- > Regards, > Li Wang
On Wed, Oct 19, 2022 at 11:17 AM Pengfei Xu <pengfei.xu@intel.com> wrote: > Hi Li Wang, > On 2022-10-19 at 10:59:54 +0800, Li Wang wrote: > > On Wed, Oct 19, 2022 at 9:40 AM Pengfei Xu <pengfei.xu@intel.com> wrote: > > > > > Hi Richard, > > > > > > On 2022-10-18 at 16:25:27 +0100, Richard Palethorpe wrote: > > > > Use memalign instead because we live in the past. > > > > > > > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > > > Cc: Pengfei Xu <pengfei.xu@intel.com> > > > > --- > > > > testcases/kernel/syscalls/ptrace/ptrace07.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c > > > b/testcases/kernel/syscalls/ptrace/ptrace07.c > > > > index 6bd2b1062..a60c2a49e 100644 > > > > --- a/testcases/kernel/syscalls/ptrace/ptrace07.c > > > > +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c > > > > @@ -35,6 +35,7 @@ > > > > #include "config.h" > > > > #include "ptrace.h" > > > > #include "tst_test.h" > > > > +#include "tst_safe_macros.h" > > > > #include "lapi/cpuid.h" > > > > > > > > #ifndef PTRACE_GETREGSET > > > > @@ -95,7 +96,7 @@ static void do_test(void) > > > > * of the XSAVE/XRSTOR save area) required by enabled features > in > > > XCR0. > > > > */ > > > > __cpuid_count(CPUID_LEAF_XSTATE, ecx, eax, ebx, ecx, edx); > > > > - xstate = aligned_alloc(64, ebx); > > > > + xstate = SAFE_MEMALIGN(64, ebx); > > > > struct iovec iov = { .iov_base = xstate, .iov_len = ebx }; > > > > int status; > > > > bool okay; > > > > > > Yes, it's better for LTP compilation. Thanks for the patch! > > > > > > I checked SAFE_MEMALIGN(), it will verify that the buffer is NULL or > not. > > > " > > > rval = memalign(alignment, size); > > > > > > if (rval == NULL) { > > > tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, > > > "memalign() failed"); > > > } > > > " > > > So could you remove below unnecessary lines in ptrace07.c, thanks. > > > > > > > I helped modify this and pushed it, thanks! > Great, thanks! > I tried that "SAFE_MEMALIGN(64, ebx);" modification in ptrace07. > Ptrace07 works well and passed in previous reproduced server. > Cool~ thanks for your feedback.
diff --git a/testcases/kernel/syscalls/ptrace/ptrace07.c b/testcases/kernel/syscalls/ptrace/ptrace07.c index 6bd2b1062..a60c2a49e 100644 --- a/testcases/kernel/syscalls/ptrace/ptrace07.c +++ b/testcases/kernel/syscalls/ptrace/ptrace07.c @@ -35,6 +35,7 @@ #include "config.h" #include "ptrace.h" #include "tst_test.h" +#include "tst_safe_macros.h" #include "lapi/cpuid.h" #ifndef PTRACE_GETREGSET @@ -95,7 +96,7 @@ static void do_test(void) * of the XSAVE/XRSTOR save area) required by enabled features in XCR0. */ __cpuid_count(CPUID_LEAF_XSTATE, ecx, eax, ebx, ecx, edx); - xstate = aligned_alloc(64, ebx); + xstate = SAFE_MEMALIGN(64, ebx); struct iovec iov = { .iov_base = xstate, .iov_len = ebx }; int status; bool okay;
Use memalign instead because we live in the past. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Cc: Pengfei Xu <pengfei.xu@intel.com> --- testcases/kernel/syscalls/ptrace/ptrace07.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)