| Message ID | 20191010081752.11334-1-pifang@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [v2] acct: fix version check on big endian system | expand |
Ping Fang <pifang@redhat.com> wrote: ac_version = ACCT_VERION | ACCT_BYTEORDER > which is 0x83 (131) on big endian system. > > failure output: > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > acct02.c:239: INFO: Verifying using 'struct acct_v3' > acct02.c:192: INFO: == entry 1 == > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:192: INFO: == entry 2 == > acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') > acct02.c:133: INFO: ac_exitcode != 32768 (0) > acct02.c:141: INFO: ac_ppid != 34501 (34500) > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:182: FAIL: end of file reached > I'm wondering that is there any necessary to use do{}...while loop for entry many times? Since the verify_acct() only check the dummy program(acct02_helper) accounting struct. So in the error logs, we could see some meaningless data(acct02) in entry 2. acct02.c:192: INFO: == entry 2 == acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') acct02.c:133: INFO: ac_exitcode != 32768 (0) acct02.c:141: INFO: ac_ppid != 34501 (34500) This needs to be fixed in a separate patch. > Signed-off-by: Ping Fang <pifang@redhat.com> > The patch itself looks good. Ack.
On Thu, Oct 10, 2019 at 4:33 PM Li Wang <liwan@redhat.com> wrote: > > Ping Fang <pifang@redhat.com> wrote: > > ac_version = ACCT_VERION | ACCT_BYTEORDER >> which is 0x83 (131) on big endian system. >> >> failure output: >> tst_kconfig.c:62: INFO: Parsing kernel config >> '/boot/config-4.18.0-147.el8.s390x' >> tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s >> tst_kconfig.c:62: INFO: Parsing kernel config >> '/boot/config-4.18.0-147.el8.s390x' >> acct02.c:239: INFO: Verifying using 'struct acct_v3' >> acct02.c:192: INFO: == entry 1 == >> acct02.c:147: INFO: ac_version != 3 (131) >> acct02.c:192: INFO: == entry 2 == >> acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') >> acct02.c:133: INFO: ac_exitcode != 32768 (0) >> acct02.c:141: INFO: ac_ppid != 34501 (34500) >> acct02.c:147: INFO: ac_version != 3 (131) >> acct02.c:182: FAIL: end of file reached >> > > I'm wondering that is there any necessary to use do{}...while loop for > entry many times? Since the verify_acct() only check the dummy > program(acct02_helper) accounting struct. So in the error logs, we could > see some meaningless data(acct02) in entry 2. > It turns out I was wrong, if accounting is turned on, system records for each terminating process but not only for that process, so we need to pick up the correct acct_struct in acct_file via the while loop. Sorry for making noise, I pull back my comments here.
Hi! > ac_version = ACCT_VERION | ACCT_BYTEORDER > which is 0x83 (131) on big endian system. > > failure output: > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > acct02.c:239: INFO: Verifying using 'struct acct_v3' > acct02.c:192: INFO: == entry 1 == > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:192: INFO: == entry 2 == > acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') > acct02.c:133: INFO: ac_exitcode != 32768 (0) > acct02.c:141: INFO: ac_ppid != 34501 (34500) > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:182: FAIL: end of file reached Looks good, acked. Also please consider a patch to the man 5 acct, it says that the version is always set to 3 for acct_v3 structure.
Hi Ping, > ac_version = ACCT_VERION | ACCT_BYTEORDER > which is 0x83 (131) on big endian system. > failure output: > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > tst_kconfig.c:62: INFO: Parsing kernel config > '/boot/config-4.18.0-147.el8.s390x' > acct02.c:239: INFO: Verifying using 'struct acct_v3' > acct02.c:192: INFO: == entry 1 == > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:192: INFO: == entry 2 == > acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') > acct02.c:133: INFO: ac_exitcode != 32768 (0) > acct02.c:141: INFO: ac_ppid != 34501 (34500) > acct02.c:147: INFO: ac_version != 3 (131) > acct02.c:182: FAIL: end of file reached > Signed-off-by: Ping Fang <pifang@redhat.com> Reviewed-by: Petr Vorel <pvorel@suse.cz> Thanks for fixing the issue. Kind regards, Petr
Hi Li, Ping, Cyril, > On Thu, Oct 10, 2019 at 4:33 PM Li Wang <liwan@redhat.com> wrote: > > Ping Fang <pifang@redhat.com> wrote: > > ac_version = ACCT_VERION | ACCT_BYTEORDER > >> which is 0x83 (131) on big endian system. > >> failure output: > >> tst_kconfig.c:62: INFO: Parsing kernel config > >> '/boot/config-4.18.0-147.el8.s390x' > >> tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > >> tst_kconfig.c:62: INFO: Parsing kernel config > >> '/boot/config-4.18.0-147.el8.s390x' > >> acct02.c:239: INFO: Verifying using 'struct acct_v3' > >> acct02.c:192: INFO: == entry 1 == > >> acct02.c:147: INFO: ac_version != 3 (131) > >> acct02.c:192: INFO: == entry 2 == > >> acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') > >> acct02.c:133: INFO: ac_exitcode != 32768 (0) > >> acct02.c:141: INFO: ac_ppid != 34501 (34500) > >> acct02.c:147: INFO: ac_version != 3 (131) > >> acct02.c:182: FAIL: end of file reached > > I'm wondering that is there any necessary to use do{}...while loop for > > entry many times? Since the verify_acct() only check the dummy > > program(acct02_helper) accounting struct. So in the error logs, we could > > see some meaningless data(acct02) in entry 2. > It turns out I was wrong, if accounting is turned on, system records for > each terminating process but not only for that process, so we need to pick > up the correct acct_struct in acct_file via the while loop. > Sorry for making noise, I pull back my comments here. Thanks for a fix, merged. Petr
Hi, > > ac_version = ACCT_VERION | ACCT_BYTEORDER > > which is 0x83 (131) on big endian system. > > failure output: > > tst_kconfig.c:62: INFO: Parsing kernel config > > '/boot/config-4.18.0-147.el8.s390x' > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > tst_kconfig.c:62: INFO: Parsing kernel config > > '/boot/config-4.18.0-147.el8.s390x' > > acct02.c:239: INFO: Verifying using 'struct acct_v3' > > acct02.c:192: INFO: == entry 1 == > > acct02.c:147: INFO: ac_version != 3 (131) > > acct02.c:192: INFO: == entry 2 == > > acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') > > acct02.c:133: INFO: ac_exitcode != 32768 (0) > > acct02.c:141: INFO: ac_ppid != 34501 (34500) > > acct02.c:147: INFO: ac_version != 3 (131) > > acct02.c:182: FAIL: end of file reached > Looks good, acked. > Also please consider a patch to the man 5 acct, it says that the version > is always set to 3 for acct_v3 structure. +1. Kind regards, Petr
diff --git a/include/lapi/acct.h b/include/lapi/acct.h index ebd65bbf4..c81b78b44 100644 --- a/include/lapi/acct.h +++ b/include/lapi/acct.h @@ -64,6 +64,11 @@ enum { ACORE = 0x08, AXSIG = 0x10 }; +# if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ +# define ACCT_BYTEORDER 0x80 +# elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +# define ACCT_BYTEORDER 0x00 +# endif #endif /* HAVE_STRUCT_ACCT_V3 */ #endif /* LAPI_ACCT_H */ diff --git a/testcases/kernel/syscalls/acct/acct02.c b/testcases/kernel/syscalls/acct/acct02.c index 7c2a27046..4d95aafec 100644 --- a/testcases/kernel/syscalls/acct/acct02.c +++ b/testcases/kernel/syscalls/acct/acct02.c @@ -142,7 +142,7 @@ static int verify_acct(void *acc, int elap_time) ret = 1; } - if (ACCT_MEMBER_V3(ac_version) != 3) { + if (ACCT_MEMBER_V3(ac_version) != (3 | ACCT_BYTEORDER)) { tst_res(TINFO, "ac_version != 3 (%d)", ACCT_MEMBER_V3(ac_version)); ret = 1;
ac_version = ACCT_VERION | ACCT_BYTEORDER which is 0x83 (131) on big endian system. failure output: tst_kconfig.c:62: INFO: Parsing kernel config '/boot/config-4.18.0-147.el8.s390x' tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s tst_kconfig.c:62: INFO: Parsing kernel config '/boot/config-4.18.0-147.el8.s390x' acct02.c:239: INFO: Verifying using 'struct acct_v3' acct02.c:192: INFO: == entry 1 == acct02.c:147: INFO: ac_version != 3 (131) acct02.c:192: INFO: == entry 2 == acct02.c:82: INFO: ac_comm != 'acct02_helper' ('acct02') acct02.c:133: INFO: ac_exitcode != 32768 (0) acct02.c:141: INFO: ac_ppid != 34501 (34500) acct02.c:147: INFO: ac_version != 3 (131) acct02.c:182: FAIL: end of file reached Signed-off-by: Ping Fang <pifang@redhat.com> --- include/lapi/acct.h | 5 +++++ testcases/kernel/syscalls/acct/acct02.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)