Message ID | a40c850a-7e99-9a67-10db-8ec2003f7f1e@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Series | Questions about failing testcase nptl/test-mutex-printers | expand |
On 03/28/2018 06:58 AM, Stefan Liebler wrote: > Does it make sense to disable lock-elision for the pretty-printer-tests? Yes it does, and that is probably the correct answer in this case. Many of the tests rely on looking at the internal details of the lock, particularly if you're going to pretty-print it, and elision does not allow that to happen because the lock and any modifications are elided. > E.g. with the following patch: > diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py > index 73ca525556..d74a8b4d4b 100644 > --- a/scripts/test_printers_common.py > +++ b/scripts/test_printers_common.py > @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, printer_names): > # Finally, load the test binary. > test('file {0}'.format(test_bin)) > > + # Disable lock elision. > + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') > + > def go_to_main(): > """Executes a gdb 'start' command, which takes us to main.""" Someone would have to confirm on at least one other arch to make sure this works as expected.
On 03/28/2018 03:15 PM, Carlos O'Donell wrote: > On 03/28/2018 06:58 AM, Stefan Liebler wrote: >> Does it make sense to disable lock-elision for the pretty-printer-tests? > > Yes it does, and that is probably the correct answer in this case. > > Many of the tests rely on looking at the internal details of the lock, > particularly if you're going to pretty-print it, and elision does not allow > that to happen because the lock and any modifications are elided. > >> E.g. with the following patch: >> diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py >> index 73ca525556..d74a8b4d4b 100644 >> --- a/scripts/test_printers_common.py >> +++ b/scripts/test_printers_common.py >> @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, printer_names): >> # Finally, load the test binary. >> test('file {0}'.format(test_bin)) >> >> + # Disable lock elision. >> + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') >> + >> def go_to_main(): >> """Executes a gdb 'start' command, which takes us to main.""" > > Someone would have to confirm on at least one other arch to make sure this works > as expected. > PING. Is there anybody with an intel / power lock-elision machine who can test this?
On 04/05/2018 08:43 AM, Stefan Liebler wrote: > On 03/28/2018 03:15 PM, Carlos O'Donell wrote: >> On 03/28/2018 06:58 AM, Stefan Liebler wrote: >>> Does it make sense to disable lock-elision for the pretty-printer-tests? >> >> Yes it does, and that is probably the correct answer in this case. >> >> Many of the tests rely on looking at the internal details of the lock, >> particularly if you're going to pretty-print it, and elision does not >> allow >> that to happen because the lock and any modifications are elided. >> >>> E.g. with the following patch: >>> diff --git a/scripts/test_printers_common.py >>> b/scripts/test_printers_common.py >>> index 73ca525556..d74a8b4d4b 100644 >>> --- a/scripts/test_printers_common.py >>> +++ b/scripts/test_printers_common.py >>> @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, >>> printer_names): >>> # Finally, load the test binary. >>> test('file {0}'.format(test_bin)) >>> >>> + # Disable lock elision. >>> + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') >>> + >>> def go_to_main(): >>> """Executes a gdb 'start' command, which takes us to main.""" >> >> Someone would have to confirm on at least one other arch to make sure >> this works >> as expected. >> > > PING. > Is there anybody with an intel / power lock-elision machine who can test > this? > PING
On 04/12/2018 03:54 PM, Stefan Liebler wrote: > On 04/05/2018 08:43 AM, Stefan Liebler wrote: >> On 03/28/2018 03:15 PM, Carlos O'Donell wrote: >>> On 03/28/2018 06:58 AM, Stefan Liebler wrote: >>>> Does it make sense to disable lock-elision for the >>>> pretty-printer-tests? >>> >>> Yes it does, and that is probably the correct answer in this case. >>> >>> Many of the tests rely on looking at the internal details of the lock, >>> particularly if you're going to pretty-print it, and elision does not >>> allow >>> that to happen because the lock and any modifications are elided. >>> >>>> E.g. with the following patch: >>>> diff --git a/scripts/test_printers_common.py >>>> b/scripts/test_printers_common.py >>>> index 73ca525556..d74a8b4d4b 100644 >>>> --- a/scripts/test_printers_common.py >>>> +++ b/scripts/test_printers_common.py >>>> @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, >>>> printer_names): >>>> # Finally, load the test binary. >>>> test('file {0}'.format(test_bin)) >>>> >>>> + # Disable lock elision. >>>> + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') >>>> + >>>> def go_to_main(): >>>> """Executes a gdb 'start' command, which takes us to main.""" >>> >>> Someone would have to confirm on at least one other arch to make sure >>> this works >>> as expected. >>> >> >> PING. >> Is there anybody with an intel / power lock-elision machine who can >> test this? >> > > PING > PING
Stefan Liebler <stli@linux.vnet.ibm.com> writes: > As I don't have access to other lock-elision enabled machines, > can somebody test this on power / intel? Tested on powerpc. Same behavior. > If I step manually to the tbegin-instruction (which starts the > transaction on s390x) and step over it, then gdb steps over the whole > transaction and we are just after the tend-instruction. That behavior also happens on powerpc with all kinds of transactions. But it's the GDB behavior with transactions on powerpc. I don't know the details on s390x. > Does it make sense to disable lock-elision for the pretty-printer-tests? > E.g. with the following patch: > diff --git a/scripts/test_printers_common.py > b/scripts/test_printers_common.py > index 73ca525556..d74a8b4d4b 100644 > --- a/scripts/test_printers_common.py > +++ b/scripts/test_printers_common.py > @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, printer_names): > # Finally, load the test binary. > test('file {0}'.format(test_bin)) > > + # Disable lock elision. > + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') > + > def go_to_main(): > """Executes a gdb 'start' command, which takes us to main.""" LGTM.
On 04/18/2018 04:40 PM, Tulio Magno Quites Machado Filho wrote: > Stefan Liebler <stli@linux.vnet.ibm.com> writes: > >> As I don't have access to other lock-elision enabled machines, >> can somebody test this on power / intel? > > Tested on powerpc. Same behavior. > >> If I step manually to the tbegin-instruction (which starts the >> transaction on s390x) and step over it, then gdb steps over the whole >> transaction and we are just after the tend-instruction. > > That behavior also happens on powerpc with all kinds of transactions. > But it's the GDB behavior with transactions on powerpc. > I don't know the details on s390x. > >> Does it make sense to disable lock-elision for the pretty-printer-tests? >> E.g. with the following patch: >> diff --git a/scripts/test_printers_common.py >> b/scripts/test_printers_common.py >> index 73ca525556..d74a8b4d4b 100644 >> --- a/scripts/test_printers_common.py >> +++ b/scripts/test_printers_common.py >> @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, printer_names): >> # Finally, load the test binary. >> test('file {0}'.format(test_bin)) >> >> + # Disable lock elision. >> + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') >> + >> def go_to_main(): >> """Executes a gdb 'start' command, which takes us to main.""" > > LGTM. > Thanks. Committed: https://sourceware.org/git/?p=glibc.git;a=commit;h=0085be1415a38b40a5a1a12e49368498f1687380
diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py index 73ca525556..d74a8b4d4b 100644 --- a/scripts/test_printers_common.py +++ b/scripts/test_printers_common.py @@ -171,6 +171,9 @@ def init_test(test_bin, printer_files, printer_names): # Finally, load the test binary. test('file {0}'.format(test_bin)) + # Disable lock elision. + test('set environment GLIBC_TUNABLES glibc.elision.enable=0') + def go_to_main(): """Executes a gdb 'start' command, which takes us to main."""