The mainline kernel can be used for relative old distros, e.g. RHEL 7.
The distro doesn't upgrade from python2 to python3, this causes the
building error that the python script is not python2 compliant.
To fix the building failure, this patch changes from the python f-string
format to traditional string format.
Reported-by: Akemi Yagi <[email protected]>
Fixes: 12fdd6c009da ("perf scripts python: Support Arm CoreSight trace data disassembly")
Signed-off-by: Leo Yan <[email protected]>
---
.../scripts/python/arm-cs-trace-disasm.py | 34 ++++++++++---------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
index 5f57d9829956..4339692a8d0b 100755
--- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
+++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
@@ -61,7 +61,7 @@ def get_optional(perf_dict, field):
def get_offset(perf_dict, field):
if field in perf_dict:
- return f"+0x{perf_dict[field]:x}"
+ return "+%#x" % perf_dict[field]
return ""
def get_dso_file_path(dso_name, dso_build_id):
@@ -76,7 +76,7 @@ def get_dso_file_path(dso_name, dso_build_id):
else:
append = "/elf"
- dso_path = f"{os.environ['PERF_BUILDID_DIR']}/{dso_name}/{dso_build_id}{append}"
+ dso_path = os.environ['PERF_BUILDID_DIR'] + "/" + dso_name + "/" + dso_build_id + append;
# Replace duplicate slash chars to single slash char
dso_path = dso_path.replace('//', '/', 1)
return dso_path
@@ -94,8 +94,8 @@ def read_disam(dso_fname, dso_start, start_addr, stop_addr):
start_addr = start_addr - dso_start;
stop_addr = stop_addr - dso_start;
disasm = [ options.objdump_name, "-d", "-z",
- f"--start-address=0x{start_addr:x}",
- f"--stop-address=0x{stop_addr:x}" ]
+ "--start-address="+format(start_addr,"#x"),
+ "--stop-address="+format(stop_addr,"#x") ]
disasm += [ dso_fname ]
disasm_output = check_output(disasm).decode('utf-8').split('\n')
disasm_cache[addr_range] = disasm_output
@@ -109,12 +109,14 @@ def print_disam(dso_fname, dso_start, start_addr, stop_addr):
m = disasm_re.search(line)
if m is None:
continue
- print(f"\t{line}")
+ print("\t" + line)
def print_sample(sample):
- print(f"Sample = {{ cpu: {sample['cpu']:04} addr: 0x{sample['addr']:016x} " \
- f"phys_addr: 0x{sample['phys_addr']:016x} ip: 0x{sample['ip']:016x} " \
- f"pid: {sample['pid']} tid: {sample['tid']} period: {sample['period']} time: {sample['time']} }}")
+ print("Sample = { cpu: %04d addr: 0x%016x phys_addr: 0x%016x ip: 0x%016x " \
+ "pid: %d tid: %d period: %d time: %d }" % \
+ (sample['cpu'], sample['addr'], sample['phys_addr'], \
+ sample['ip'], sample['pid'], sample['tid'], \
+ sample['period'], sample['time']))
def trace_begin():
print('ARM CoreSight Trace Data Assembler Dump')
@@ -131,7 +133,7 @@ def common_start_str(comm, sample):
cpu = sample["cpu"]
pid = sample["pid"]
tid = sample["tid"]
- return f"{comm:>16} {pid:>5}/{tid:<5} [{cpu:04}] {sec:9}.{ns:09} "
+ return "%16s %5u/%-5u [%04u] %9u.%09u " % (comm, pid, tid, cpu, sec, ns)
# This code is copied from intel-pt-events.py for printing source code
# line and symbols.
@@ -171,7 +173,7 @@ def print_srccode(comm, param_dict, sample, symbol, dso):
glb_line_number = line_number
glb_source_file_name = source_file_name
- print(f"{start_str}{src_str}")
+ print(start_str, src_str)
def process_event(param_dict):
global cache_size
@@ -188,7 +190,7 @@ def process_event(param_dict):
symbol = get_optional(param_dict, "symbol")
if (options.verbose == True):
- print(f"Event type: {name}")
+ print("Event type: %s" % name)
print_sample(sample)
# If cannot find dso so cannot dump assembler, bail out
@@ -197,7 +199,7 @@ def process_event(param_dict):
# Validate dso start and end addresses
if ((dso_start == '[unknown]') or (dso_end == '[unknown]')):
- print(f"Failed to find valid dso map for dso {dso}")
+ print("Failed to find valid dso map for dso %s" % dso)
return
if (name[0:12] == "instructions"):
@@ -244,15 +246,15 @@ def process_event(param_dict):
# Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4
if (start_addr == 0 and stop_addr == 4):
- print(f"CPU{cpu}: CS_ETM_TRACE_ON packet is inserted")
+ print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
return
if (start_addr < int(dso_start) or start_addr > int(dso_end)):
- print(f"Start address 0x{start_addr:x} is out of range [ 0x{dso_start:x} .. 0x{dso_end:x} ] for dso {dso}")
+ print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso))
return
if (stop_addr < int(dso_start) or stop_addr > int(dso_end)):
- print(f"Stop address 0x{stop_addr:x} is out of range [ 0x{dso_start:x} .. 0x{dso_end:x} ] for dso {dso}")
+ print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso))
return
if (options.objdump_name != None):
@@ -267,6 +269,6 @@ def process_event(param_dict):
if path.exists(dso_fname):
print_disam(dso_fname, dso_vm_start, start_addr, stop_addr)
else:
- print(f"Failed to find dso {dso} for address range [ 0x{start_addr:x} .. 0x{stop_addr:x} ]")
+ print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
print_srccode(comm, param_dict, sample, symbol, dso)
--
2.25.1
Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> The distro doesn't upgrade from python2 to python3, this causes the
> building error that the python script is not python2 compliant.
>
> To fix the building failure, this patch changes from the python f-string
> format to traditional string format.
Thanks, applied.
- Arnaldo
> Reported-by: Akemi Yagi <[email protected]>
> Fixes: 12fdd6c009da ("perf scripts python: Support Arm CoreSight trace data disassembly")
> Signed-off-by: Leo Yan <[email protected]>
> ---
> .../scripts/python/arm-cs-trace-disasm.py | 34 ++++++++++---------
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> index 5f57d9829956..4339692a8d0b 100755
> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> @@ -61,7 +61,7 @@ def get_optional(perf_dict, field):
>
> def get_offset(perf_dict, field):
> if field in perf_dict:
> - return f"+0x{perf_dict[field]:x}"
> + return "+%#x" % perf_dict[field]
> return ""
>
> def get_dso_file_path(dso_name, dso_build_id):
> @@ -76,7 +76,7 @@ def get_dso_file_path(dso_name, dso_build_id):
> else:
> append = "/elf"
>
> - dso_path = f"{os.environ['PERF_BUILDID_DIR']}/{dso_name}/{dso_build_id}{append}"
> + dso_path = os.environ['PERF_BUILDID_DIR'] + "/" + dso_name + "/" + dso_build_id + append;
> # Replace duplicate slash chars to single slash char
> dso_path = dso_path.replace('//', '/', 1)
> return dso_path
> @@ -94,8 +94,8 @@ def read_disam(dso_fname, dso_start, start_addr, stop_addr):
> start_addr = start_addr - dso_start;
> stop_addr = stop_addr - dso_start;
> disasm = [ options.objdump_name, "-d", "-z",
> - f"--start-address=0x{start_addr:x}",
> - f"--stop-address=0x{stop_addr:x}" ]
> + "--start-address="+format(start_addr,"#x"),
> + "--stop-address="+format(stop_addr,"#x") ]
> disasm += [ dso_fname ]
> disasm_output = check_output(disasm).decode('utf-8').split('\n')
> disasm_cache[addr_range] = disasm_output
> @@ -109,12 +109,14 @@ def print_disam(dso_fname, dso_start, start_addr, stop_addr):
> m = disasm_re.search(line)
> if m is None:
> continue
> - print(f"\t{line}")
> + print("\t" + line)
>
> def print_sample(sample):
> - print(f"Sample = {{ cpu: {sample['cpu']:04} addr: 0x{sample['addr']:016x} " \
> - f"phys_addr: 0x{sample['phys_addr']:016x} ip: 0x{sample['ip']:016x} " \
> - f"pid: {sample['pid']} tid: {sample['tid']} period: {sample['period']} time: {sample['time']} }}")
> + print("Sample = { cpu: %04d addr: 0x%016x phys_addr: 0x%016x ip: 0x%016x " \
> + "pid: %d tid: %d period: %d time: %d }" % \
> + (sample['cpu'], sample['addr'], sample['phys_addr'], \
> + sample['ip'], sample['pid'], sample['tid'], \
> + sample['period'], sample['time']))
>
> def trace_begin():
> print('ARM CoreSight Trace Data Assembler Dump')
> @@ -131,7 +133,7 @@ def common_start_str(comm, sample):
> cpu = sample["cpu"]
> pid = sample["pid"]
> tid = sample["tid"]
> - return f"{comm:>16} {pid:>5}/{tid:<5} [{cpu:04}] {sec:9}.{ns:09} "
> + return "%16s %5u/%-5u [%04u] %9u.%09u " % (comm, pid, tid, cpu, sec, ns)
>
> # This code is copied from intel-pt-events.py for printing source code
> # line and symbols.
> @@ -171,7 +173,7 @@ def print_srccode(comm, param_dict, sample, symbol, dso):
> glb_line_number = line_number
> glb_source_file_name = source_file_name
>
> - print(f"{start_str}{src_str}")
> + print(start_str, src_str)
>
> def process_event(param_dict):
> global cache_size
> @@ -188,7 +190,7 @@ def process_event(param_dict):
> symbol = get_optional(param_dict, "symbol")
>
> if (options.verbose == True):
> - print(f"Event type: {name}")
> + print("Event type: %s" % name)
> print_sample(sample)
>
> # If cannot find dso so cannot dump assembler, bail out
> @@ -197,7 +199,7 @@ def process_event(param_dict):
>
> # Validate dso start and end addresses
> if ((dso_start == '[unknown]') or (dso_end == '[unknown]')):
> - print(f"Failed to find valid dso map for dso {dso}")
> + print("Failed to find valid dso map for dso %s" % dso)
> return
>
> if (name[0:12] == "instructions"):
> @@ -244,15 +246,15 @@ def process_event(param_dict):
>
> # Handle CS_ETM_TRACE_ON packet if start_addr=0 and stop_addr=4
> if (start_addr == 0 and stop_addr == 4):
> - print(f"CPU{cpu}: CS_ETM_TRACE_ON packet is inserted")
> + print("CPU%d: CS_ETM_TRACE_ON packet is inserted" % cpu)
> return
>
> if (start_addr < int(dso_start) or start_addr > int(dso_end)):
> - print(f"Start address 0x{start_addr:x} is out of range [ 0x{dso_start:x} .. 0x{dso_end:x} ] for dso {dso}")
> + print("Start address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (start_addr, int(dso_start), int(dso_end), dso))
> return
>
> if (stop_addr < int(dso_start) or stop_addr > int(dso_end)):
> - print(f"Stop address 0x{stop_addr:x} is out of range [ 0x{dso_start:x} .. 0x{dso_end:x} ] for dso {dso}")
> + print("Stop address 0x%x is out of range [ 0x%x .. 0x%x ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso))
> return
>
> if (options.objdump_name != None):
> @@ -267,6 +269,6 @@ def process_event(param_dict):
> if path.exists(dso_fname):
> print_disam(dso_fname, dso_vm_start, start_addr, stop_addr)
> else:
> - print(f"Failed to find dso {dso} for address range [ 0x{start_addr:x} .. 0x{stop_addr:x} ]")
> + print("Failed to find dso %s for address range [ 0x%x .. 0x%x ]" % (dso, start_addr, stop_addr))
>
> print_srccode(comm, param_dict, sample, symbol, dso)
> --
> 2.25.1
--
- Arnaldo
On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
>
> Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > The distro doesn't upgrade from python2 to python3, this causes the
> > building error that the python script is not python2 compliant.
> >
> > To fix the building failure, this patch changes from the python f-string
> > format to traditional string format.
>
> Thanks, applied.
>
> - Arnaldo
Leo / Arnaldo,
Applying the patch on top of -5.19-rc8 fixes the problem that we (the
ELRepo Project) experienced when attempting to build on RHEL7.
So --
Tested-by: Alan Bartlett <[email protected]>
Hopefully you will get it to Linus in time for -5.19 GA.
Thanks,
Alan.
On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
>
> On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> >
> > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > The distro doesn't upgrade from python2 to python3, this causes the
> > > building error that the python script is not python2 compliant.
> > >
> > > To fix the building failure, this patch changes from the python f-string
> > > format to traditional string format.
> >
> > Thanks, applied.
> >
> > - Arnaldo
>
> Leo / Arnaldo,
>
> Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> ELRepo Project) experienced when attempting to build on RHEL7.
>
> So --
>
> Tested-by: Alan Bartlett <[email protected]>
>
> Hopefully you will get it to Linus in time for -5.19 GA.
>
> Thanks,
> Alan.
So I'm somewhat concerned about perf supporting unsupported
distributions and this holding the code base back. RHEL7 was launched
8 years ago (June 10, 2014) and full support ended 3 years ago (August
6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
Maintenance Support 2" phase which is defined to mean [2]:
```
During the Maintenance Support Phase for Red Hat Enterprise Linux
Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
Linux version 7, Red Hat defined Critical and Important impact
Security Advisories (RHSAs) and selected (at Red Hat discretion)
Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
become available. Other errata advisories may be delivered as
appropriate.
New functionality and new hardware enablement are not planned for
availability in the Maintenance Support (RHEL 8 & 9) Phase and
Maintenance Support 2 (RHEL 7) Phase.
```
From this definition, why would RHEL7 pick up a new perf tool? I don't
think they would and as such we don't need to worry about supporting
it. RHEL8 defaults to python 3 and full support ends for it next year.
Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
this in future. I think the bar for caring should be "will the distro
pick up our code", if we don't do this then we're signing up to not
allowing tools to update for 10 years! If someone is building a kernel
and perf tool on RHEL7 then they should be signing up to also deal
with tool chain issues, which in this case can mean installing
python3.
Thanks,
Ian
[1] https://access.redhat.com/product-life-cycles/
[2] https://access.redhat.com/support/policy/updates/errata
Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> >
> > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > >
> > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > building error that the python script is not python2 compliant.
> > > >
> > > > To fix the building failure, this patch changes from the python f-string
> > > > format to traditional string format.
> > >
> > > Thanks, applied.
> > >
> > > - Arnaldo
> >
> > Leo / Arnaldo,
> >
> > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > ELRepo Project) experienced when attempting to build on RHEL7.
> >
> > So --
> >
> > Tested-by: Alan Bartlett <[email protected]>
> >
> > Hopefully you will get it to Linus in time for -5.19 GA.
> So I'm somewhat concerned about perf supporting unsupported
> distributions and this holding the code base back. RHEL7 was launched
> 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> Maintenance Support 2" phase which is defined to mean [2]:
>
> ```
> During the Maintenance Support Phase for Red Hat Enterprise Linux
> Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> Linux version 7, Red Hat defined Critical and Important impact
> Security Advisories (RHSAs) and selected (at Red Hat discretion)
> Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> become available. Other errata advisories may be delivered as
> appropriate.
>
> New functionality and new hardware enablement are not planned for
> availability in the Maintenance Support (RHEL 8 & 9) Phase and
> Maintenance Support 2 (RHEL 7) Phase.
> ```
>
> >From this definition, why would RHEL7 pick up a new perf tool? I don't
> think they would and as such we don't need to worry about supporting
> it. RHEL8 defaults to python 3 and full support ends for it next year.
> Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> this in future. I think the bar for caring should be "will the distro
> pick up our code", if we don't do this then we're signing up to not
> allowing tools to update for 10 years! If someone is building a kernel
> and perf tool on RHEL7 then they should be signing up to also deal
> with tool chain issues, which in this case can mean installing
> python3.
In this specific supporting things that people report using, like was
done in this case, isn't such a big problem.
Someone reported a problem in a system they used, the author of the code
in question posted a patch allowing perf to be used in such old systems,
doesn't get in the way of newer systems, small patch, merged, life goes
on.
Sometimes some organizations are stuck with some distro till they can go
thru re-certifications, bidding for new hardware, whatever, and then
they want to continue using the latest perf on those systems because
they want to benefit from new features we're working on that work on
such systems. If the cost is small, like in this case, I see no problems
to have perf working on such older systems.
- Arnaldo
On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > So I'm somewhat concerned about perf supporting unsupported
> > distributions and this holding the code base back. RHEL7 was launched
> > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > Maintenance Support 2" phase which is defined to mean [2]:
[...]
> In this specific supporting things that people report using, like was
> done in this case, isn't such a big problem.
>
> Someone reported a problem in a system they used, the author of the code
> in question posted a patch allowing perf to be used in such old systems,
> doesn't get in the way of newer systems, small patch, merged, life goes
> on.
>
> Sometimes some organizations are stuck with some distro till they can go
> thru re-certifications, bidding for new hardware, whatever, and then
> they want to continue using the latest perf on those systems because
> they want to benefit from new features we're working on that work on
> such systems. If the cost is small, like in this case, I see no problems
> to have perf working on such older systems.
>
> - Arnaldo
Just wanted to make a note about the "old" systems.
While RHEL 7 might be regarded as "old" in general, it may not be so
in the world of Enterprise Linux. A graph of EPEL mirror stats [1],
while it is from about a year ago, shows EL 7 (RHEL 7 and its
rebuilds) has a huge user base and was still growing quite fast.
By the way, my main workstation runs RHEL 7. ;-)
Akemi
[1] https://twitter.com/mattdm/status/1447224008831811588
On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > >
> > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > >
> > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > building error that the python script is not python2 compliant.
> > > > >
> > > > > To fix the building failure, this patch changes from the python f-string
> > > > > format to traditional string format.
> > > >
> > > > Thanks, applied.
> > > >
> > > > - Arnaldo
> > >
> > > Leo / Arnaldo,
> > >
> > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > ELRepo Project) experienced when attempting to build on RHEL7.
> > >
> > > So --
> > >
> > > Tested-by: Alan Bartlett <[email protected]>
> > >
> > > Hopefully you will get it to Linus in time for -5.19 GA.
>
> > So I'm somewhat concerned about perf supporting unsupported
> > distributions and this holding the code base back. RHEL7 was launched
> > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > Maintenance Support 2" phase which is defined to mean [2]:
> >
> > ```
> > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > Linux version 7, Red Hat defined Critical and Important impact
> > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > become available. Other errata advisories may be delivered as
> > appropriate.
> >
> > New functionality and new hardware enablement are not planned for
> > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > Maintenance Support 2 (RHEL 7) Phase.
> > ```
> >
> > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > think they would and as such we don't need to worry about supporting
> > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > this in future. I think the bar for caring should be "will the distro
> > pick up our code", if we don't do this then we're signing up to not
> > allowing tools to update for 10 years! If someone is building a kernel
> > and perf tool on RHEL7 then they should be signing up to also deal
> > with tool chain issues, which in this case can mean installing
> > python3.
>
> In this specific supporting things that people report using, like was
> done in this case, isn't such a big problem.
So there are linters will fire for this code and say it is not
pythonic. It is only a linter warning vs asking to support an 8 year
old out of support distribution. There are other cases, such as
improving the C code structure, where we've failed to land changes
because of build errors on old distributions. This could indicate perf
code is wrong or the distribution is wrong. I'm saying that if we
believe in the perf code being correct and the distribution is out of
support, then we should keep the perf code as-is and the issue is one
for user of the out-of-support distribution.
> Someone reported a problem in a system they used, the author of the code
> in question posted a patch allowing perf to be used in such old systems,
> doesn't get in the way of newer systems, small patch, merged, life goes
> on.
Right, but we're setting a precedent for supporting out of support
distributions. If we can say "life goes on" can we land this *current*
Debian fix?
https://lore.kernel.org/lkml/[email protected]/
> Sometimes some organizations are stuck with some distro till they can go
> thru re-certifications, bidding for new hardware, whatever, and then
> they want to continue using the latest perf on those systems because
> they want to benefit from new features we're working on that work on
> such systems. If the cost is small, like in this case, I see no problems
> to have perf working on such older systems.
So there's no problem with perf working on old systems. The issue is
supporting 10 year old unsupported build infrastructure. The fact that
the build infrastructure is unsupported means we need to carry all the
fixes in the tools tree and that can mean doing some questionably sane
things, like supporting python 2 (end of life for 2.5 years) on RHEL7
(end of full support 3 years ago). RHEL8 still has a year of support,
so great test that. RHEL7 then fix your tools and perf will work for
you - where fix means "rpm -i python3", hardly a huge chore.
Thanks,
Ian
On Tue, Jul 26, 2022 at 1:35 PM Akemi Yagi <[email protected]> wrote:
>
> On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
>
> > > So I'm somewhat concerned about perf supporting unsupported
> > > distributions and this holding the code base back. RHEL7 was launched
> > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > Maintenance Support 2" phase which is defined to mean [2]:
> [...]
>
> > In this specific supporting things that people report using, like was
> > done in this case, isn't such a big problem.
> >
> > Someone reported a problem in a system they used, the author of the code
> > in question posted a patch allowing perf to be used in such old systems,
> > doesn't get in the way of newer systems, small patch, merged, life goes
> > on.
> >
> > Sometimes some organizations are stuck with some distro till they can go
> > thru re-certifications, bidding for new hardware, whatever, and then
> > they want to continue using the latest perf on those systems because
> > they want to benefit from new features we're working on that work on
> > such systems. If the cost is small, like in this case, I see no problems
> > to have perf working on such older systems.
> >
> > - Arnaldo
>
> Just wanted to make a note about the "old" systems.
>
> While RHEL 7 might be regarded as "old" in general, it may not be so
> in the world of Enterprise Linux. A graph of EPEL mirror stats [1],
> while it is from about a year ago, shows EL 7 (RHEL 7 and its
> rebuilds) has a huge user base and was still growing quite fast.
So if RedHat don't care to support it, why as developers should we?
> By the way, my main workstation runs RHEL 7. ;-)
So you need new tools that are broken so you can give pressure to
whoever is subjecting you to out-of-date distributions to pull their
fingers out and fix this ;-) You may also want to think about the
security of your system.
Thanks,
Ian
> Akemi
>
> [1] https://twitter.com/mattdm/status/1447224008831811588
Em Tue, Jul 26, 2022 at 01:43:31PM -0700, Ian Rogers escreveu:
> On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > > >
> > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > >
> > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > building error that the python script is not python2 compliant.
> > > > > >
> > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > format to traditional string format.
> > > > >
> > > > > Thanks, applied.
> > > > >
> > > > > - Arnaldo
> > > >
> > > > Leo / Arnaldo,
> > > >
> > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > >
> > > > So --
> > > >
> > > > Tested-by: Alan Bartlett <[email protected]>
> > > >
> > > > Hopefully you will get it to Linus in time for -5.19 GA.
> >
> > > So I'm somewhat concerned about perf supporting unsupported
> > > distributions and this holding the code base back. RHEL7 was launched
> > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > Maintenance Support 2" phase which is defined to mean [2]:
> > >
> > > ```
> > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > Linux version 7, Red Hat defined Critical and Important impact
> > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > become available. Other errata advisories may be delivered as
> > > appropriate.
> > >
> > > New functionality and new hardware enablement are not planned for
> > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > Maintenance Support 2 (RHEL 7) Phase.
> > > ```
> > >
> > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > think they would and as such we don't need to worry about supporting
> > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > this in future. I think the bar for caring should be "will the distro
> > > pick up our code", if we don't do this then we're signing up to not
> > > allowing tools to update for 10 years! If someone is building a kernel
> > > and perf tool on RHEL7 then they should be signing up to also deal
> > > with tool chain issues, which in this case can mean installing
> > > python3.
> >
> > In this specific supporting things that people report using, like was
> > done in this case, isn't such a big problem.
>
> So there are linters will fire for this code and say it is not
> pythonic. It is only a linter warning vs asking to support an 8 year
> old out of support distribution. There are other cases, such as
> improving the C code structure, where we've failed to land changes
> because of build errors on old distributions. This could indicate perf
> code is wrong or the distribution is wrong. I'm saying that if we
> believe in the perf code being correct and the distribution is out of
> support, then we should keep the perf code as-is and the issue is one
> for user of the out-of-support distribution.
>
> > Someone reported a problem in a system they used, the author of the code
> > in question posted a patch allowing perf to be used in such old systems,
> > doesn't get in the way of newer systems, small patch, merged, life goes
> > on.
>
> Right, but we're setting a precedent for supporting out of support
> distributions. If we can say "life goes on" can we land this *current*
> Debian fix?
> https://lore.kernel.org/lkml/[email protected]/
I'll revisit the discussion with PeterZ...
- Arnaldo
> > Sometimes some organizations are stuck with some distro till they can go
> > thru re-certifications, bidding for new hardware, whatever, and then
> > they want to continue using the latest perf on those systems because
> > they want to benefit from new features we're working on that work on
> > such systems. If the cost is small, like in this case, I see no problems
> > to have perf working on such older systems.
>
> So there's no problem with perf working on old systems. The issue is
> supporting 10 year old unsupported build infrastructure. The fact that
> the build infrastructure is unsupported means we need to carry all the
> fixes in the tools tree and that can mean doing some questionably sane
> things, like supporting python 2 (end of life for 2.5 years) on RHEL7
> (end of full support 3 years ago). RHEL8 still has a year of support,
> so great test that. RHEL7 then fix your tools and perf will work for
> you - where fix means "rpm -i python3", hardly a huge chore.
>
> Thanks,
> Ian
--
- Arnaldo
On Tue, Jul 26, 2022 at 01:35:40PM -0700, Akemi Yagi wrote:
> By the way, my main workstation runs RHEL 7. ;-)
That seems highly irresponsible, given the amount of security bugs fixed
by all the relevant projects.
On 7/26/22 4:43 PM, Ian Rogers wrote:
> On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > > >
> > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > >
> > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > building error that the python script is not python2 compliant.
> > > > > >
> > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > format to traditional string format.
> > > > >
> > > > > Thanks, applied.
> > > > >
> > > > > - Arnaldo
> > > >
> > > > Leo / Arnaldo,
> > > >
> > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > >
> > > > So --
> > > >
> > > > Tested-by: Alan Bartlett <[email protected]>
> > > >
> > > > Hopefully you will get it to Linus in time for -5.19 GA.
> >
> > > So I'm somewhat concerned about perf supporting unsupported
> > > distributions and this holding the code base back. RHEL7 was launched
> > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > Maintenance Support 2" phase which is defined to mean [2]:
> > >
> > > ```
> > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > Linux version 7, Red Hat defined Critical and Important impact
> > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > become available. Other errata advisories may be delivered as
> > > appropriate.
> > >
> > > New functionality and new hardware enablement are not planned for
> > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > Maintenance Support 2 (RHEL 7) Phase.
> > > ```
> > >
> > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > think they would and as such we don't need to worry about supporting
> > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > this in future. I think the bar for caring should be "will the distro
> > > pick up our code", if we don't do this then we're signing up to not
> > > allowing tools to update for 10 years! If someone is building a kernel
> > > and perf tool on RHEL7 then they should be signing up to also deal
> > > with tool chain issues, which in this case can mean installing
> > > python3.
> >
> > In this specific supporting things that people report using, like was
> > done in this case, isn't such a big problem.
>
> So there are linters will fire for this code and say it is not
> pythonic. It is only a linter warning vs asking to support an 8 year
> old out of support distribution. There are other cases, such as
> improving the C code structure, where we've failed to land changes
> because of build errors on old distributions. This could indicate perf
> code is wrong or the distribution is wrong. I'm saying that if we
> believe in the perf code being correct and the distribution is out of
> support, then we should keep the perf code as-is and the issue is one
> for user of the out-of-support distribution.
>
> > Someone reported a problem in a system they used, the author of the code
> > in question posted a patch allowing perf to be used in such old systems,
> > doesn't get in the way of newer systems, small patch, merged, life goes
> > on.
Considering the proposed patch, can you be sure that replacing the
f-string format with the legacy format won't cause a regression for
some python3 user somewhere when this hits the real world? Even
if it does not cause a regression today, as new versions and features
are added to python3, can you be sure none of those new features
will depend on the upgrade from the legacy format to the f-string
format here to work properly? So many regressions happen because
the people who write patches cannot possibly foresee how their
patch is going to affect the millions of Linux users out there, but still
they are certain it will not cause a regression somewhere. So how
can the chances that this patch will cause a regression be minimized?
It seems to me for this to be suitable for the Linux kernel, the
default should be to use the modern python3 format and only
enable python2 compatibility via a sysctl setting and/or kernel boot
option for those who are still using python2. There should be no
change to the behavior of the kernel for users who have upgraded
to python3. But I don't see any such consideration for python3
users in this patch.
>
> Right, but we're setting a precedent for supporting out of support
> distributions. If we can say "life goes on" can we land this *current*
> Debian fix?
> https://lore.kernel.org/lkml/[email protected]/
>
> > Sometimes some organizations are stuck with some distro till they can go
> > thru re-certifications, bidding for new hardware, whatever, and then
> > they want to continue using the latest perf on those systems because
> > they want to benefit from new features we're working on that work on
> > such systems. If the cost is small, like in this case, I see no problems
> > to have perf working on such older systems.
>
> So there's no problem with perf working on old systems. The issue is
> supporting 10 year old unsupported build infrastructure. The fact that
> the build infrastructure is unsupported means we need to carry all the
> fixes in the tools tree and that can mean doing some questionably sane
> things, like supporting python 2 (end of life for 2.5 years) on RHEL7
> (end of full support 3 years ago). RHEL8 still has a year of support,
> so great test that. RHEL7 then fix your tools and perf will work for
> you - where fix means "rpm -i python3", hardly a huge chore.
In large projects, going from python2 to 3 probably involves
a bit more than doing just that and expecting everything to work.
That said, large projects should have the resources to upgrade
to python3 on "old" enterprise systems. So I more or less agree
with Ian here.
Regards,
Chuck
On 8/17/2022 3:52 PM, Chuck Zmudzinski wrote:
> On 7/26/22 4:43 PM, Ian Rogers wrote:
> > On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> > >
> > > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > > > >
> > > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > > >
> > > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > > building error that the python script is not python2 compliant.
> > > > > > >
> > > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > > format to traditional string format.
> > > > > >
> > > > > > Thanks, applied.
> > > > > >
> > > > > > - Arnaldo
> > > > >
> > > > > Leo / Arnaldo,
> > > > >
> > > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > > >
> > > > > So --
> > > > >
> > > > > Tested-by: Alan Bartlett <[email protected]>
> > > > >
> > > > > Hopefully you will get it to Linus in time for -5.19 GA.
> > >
> > > > So I'm somewhat concerned about perf supporting unsupported
> > > > distributions and this holding the code base back. RHEL7 was launched
> > > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > > Maintenance Support 2" phase which is defined to mean [2]:
> > > >
> > > > ```
> > > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > > Linux version 7, Red Hat defined Critical and Important impact
> > > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > > become available. Other errata advisories may be delivered as
> > > > appropriate.
> > > >
> > > > New functionality and new hardware enablement are not planned for
> > > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > > Maintenance Support 2 (RHEL 7) Phase.
> > > > ```
> > > >
> > > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > > think they would and as such we don't need to worry about supporting
> > > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > > this in future. I think the bar for caring should be "will the distro
> > > > pick up our code", if we don't do this then we're signing up to not
> > > > allowing tools to update for 10 years! If someone is building a kernel
> > > > and perf tool on RHEL7 then they should be signing up to also deal
> > > > with tool chain issues, which in this case can mean installing
> > > > python3.
> > >
> > > In this specific supporting things that people report using, like was
> > > done in this case, isn't such a big problem.
> >
> > So there are linters will fire for this code and say it is not
> > pythonic. It is only a linter warning vs asking to support an 8 year
> > old out of support distribution. There are other cases, such as
> > improving the C code structure, where we've failed to land changes
> > because of build errors on old distributions. This could indicate perf
> > code is wrong or the distribution is wrong. I'm saying that if we
> > believe in the perf code being correct and the distribution is out of
> > support, then we should keep the perf code as-is and the issue is one
> > for user of the out-of-support distribution.
> >
> > > Someone reported a problem in a system they used, the author of the code
> > > in question posted a patch allowing perf to be used in such old systems,
> > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > on.
>
> Considering the proposed patch, can you be sure that replacing the
> f-string format with the legacy format won't cause a regression for
> some python3 user somewhere when this hits the real world? Even
> if it does not cause a regression today, as new versions and features
> are added to python3, can you be sure none of those new features
> will depend on the upgrade from the legacy format to the f-string
> format here to work properly? So many regressions happen because
> the people who write patches cannot possibly foresee how their
> patch is going to affect the millions of Linux users out there, but still
> they are certain it will not cause a regression somewhere. So how
> can the chances that this patch will cause a regression be minimized?
>
> It seems to me for this to be suitable for the Linux kernel, the
> default should be to use the modern python3 format and only
> enable python2 compatibility via a sysctl setting and/or kernel boot
> option for those who are still using python2. There should be no
> change to the behavior of the kernel for users who have upgraded
> to python3. But I don't see any such consideration for python3
> users in this patch.
Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
does not apply. But can't the script implement this simple logic:
If python version = 3 use f-string format
if python version = 2 use traditional string format
Best regards,
Chuck
On Wed, Aug 17, 2022 at 3:13 PM Chuck Zmudzinski <[email protected]> wrote:
>
> On 8/17/2022 3:52 PM, Chuck Zmudzinski wrote:
> > On 7/26/22 4:43 PM, Ian Rogers wrote:
> > > On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> > > <[email protected]> wrote:
> > > >
> > > > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > > > > >
> > > > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > > > >
> > > > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > > > building error that the python script is not python2 compliant.
> > > > > > > >
> > > > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > > > format to traditional string format.
> > > > > > >
> > > > > > > Thanks, applied.
> > > > > > >
> > > > > > > - Arnaldo
> > > > > >
> > > > > > Leo / Arnaldo,
> > > > > >
> > > > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > > > >
> > > > > > So --
> > > > > >
> > > > > > Tested-by: Alan Bartlett <[email protected]>
> > > > > >
> > > > > > Hopefully you will get it to Linus in time for -5.19 GA.
> > > >
> > > > > So I'm somewhat concerned about perf supporting unsupported
> > > > > distributions and this holding the code base back. RHEL7 was launched
> > > > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > > > Maintenance Support 2" phase which is defined to mean [2]:
> > > > >
> > > > > ```
> > > > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > > > Linux version 7, Red Hat defined Critical and Important impact
> > > > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > > > become available. Other errata advisories may be delivered as
> > > > > appropriate.
> > > > >
> > > > > New functionality and new hardware enablement are not planned for
> > > > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > > > Maintenance Support 2 (RHEL 7) Phase.
> > > > > ```
> > > > >
> > > > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > > > think they would and as such we don't need to worry about supporting
> > > > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > > > this in future. I think the bar for caring should be "will the distro
> > > > > pick up our code", if we don't do this then we're signing up to not
> > > > > allowing tools to update for 10 years! If someone is building a kernel
> > > > > and perf tool on RHEL7 then they should be signing up to also deal
> > > > > with tool chain issues, which in this case can mean installing
> > > > > python3.
> > > >
> > > > In this specific supporting things that people report using, like was
> > > > done in this case, isn't such a big problem.
> > >
> > > So there are linters will fire for this code and say it is not
> > > pythonic. It is only a linter warning vs asking to support an 8 year
> > > old out of support distribution. There are other cases, such as
> > > improving the C code structure, where we've failed to land changes
> > > because of build errors on old distributions. This could indicate perf
> > > code is wrong or the distribution is wrong. I'm saying that if we
> > > believe in the perf code being correct and the distribution is out of
> > > support, then we should keep the perf code as-is and the issue is one
> > > for user of the out-of-support distribution.
> > >
> > > > Someone reported a problem in a system they used, the author of the code
> > > > in question posted a patch allowing perf to be used in such old systems,
> > > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > > on.
> >
> > Considering the proposed patch, can you be sure that replacing the
> > f-string format with the legacy format won't cause a regression for
> > some python3 user somewhere when this hits the real world? Even
> > if it does not cause a regression today, as new versions and features
> > are added to python3, can you be sure none of those new features
> > will depend on the upgrade from the legacy format to the f-string
> > format here to work properly? So many regressions happen because
> > the people who write patches cannot possibly foresee how their
> > patch is going to affect the millions of Linux users out there, but still
> > they are certain it will not cause a regression somewhere. So how
> > can the chances that this patch will cause a regression be minimized?
> >
> > It seems to me for this to be suitable for the Linux kernel, the
> > default should be to use the modern python3 format and only
> > enable python2 compatibility via a sysctl setting and/or kernel boot
> > option for those who are still using python2. There should be no
> > change to the behavior of the kernel for users who have upgraded
> > to python3. But I don't see any such consideration for python3
> > users in this patch.
>
> Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
> does not apply. But can't the script implement this simple logic:
>
> If python version = 3 use f-string format
> if python version = 2 use traditional string format
Doing this in the script would be noisy, having two scripts less than
ideal. I'd suggest we wait two weeks, declare the official death of
RHEL7 without "rpm -i python3" and then revert the python3 to python2
patch. There are plenty of things to worry about and python2 shouldn't
be one of them (it died over 2 years ago).
Thanks,
Ian
> Best regards,
>
> Chuck
On 8/17/22 6:36 PM, Ian Rogers wrote:
> On Wed, Aug 17, 2022 at 3:13 PM Chuck Zmudzinski <[email protected]> wrote:
> >
> > On 8/17/2022 3:52 PM, Chuck Zmudzinski wrote:
> > > On 7/26/22 4:43 PM, Ian Rogers wrote:
> > > > On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> > > > <[email protected]> wrote:
> > > > >
> > > > > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > > > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <[email protected]> wrote:
> > > > > > >
> > > > > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > > > > >
> > > > > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > > > > building error that the python script is not python2 compliant.
> > > > > > > > >
> > > > > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > > > > format to traditional string format.
> > > > > > > >
> > > > > > > > Thanks, applied.
> > > > > > > >
> > > > > > > > - Arnaldo
> > > > > > >
> > > > > > > Leo / Arnaldo,
> > > > > > >
> > > > > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > > > > >
> > > > > > > So --
> > > > > > >
> > > > > > > Tested-by: Alan Bartlett <[email protected]>
> > > > > > >
> > > > > > > Hopefully you will get it to Linus in time for -5.19 GA.
> > > > >
> > > > > > So I'm somewhat concerned about perf supporting unsupported
> > > > > > distributions and this holding the code base back. RHEL7 was launched
> > > > > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > > > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > > > > Maintenance Support 2" phase which is defined to mean [2]:
> > > > > >
> > > > > > ```
> > > > > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > > > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > > > > Linux version 7, Red Hat defined Critical and Important impact
> > > > > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > > > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > > > > become available. Other errata advisories may be delivered as
> > > > > > appropriate.
> > > > > >
> > > > > > New functionality and new hardware enablement are not planned for
> > > > > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > > > > Maintenance Support 2 (RHEL 7) Phase.
> > > > > > ```
> > > > > >
> > > > > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > > > > think they would and as such we don't need to worry about supporting
> > > > > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > > > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > > > > this in future. I think the bar for caring should be "will the distro
> > > > > > pick up our code", if we don't do this then we're signing up to not
> > > > > > allowing tools to update for 10 years! If someone is building a kernel
> > > > > > and perf tool on RHEL7 then they should be signing up to also deal
> > > > > > with tool chain issues, which in this case can mean installing
> > > > > > python3.
> > > > >
> > > > > In this specific supporting things that people report using, like was
> > > > > done in this case, isn't such a big problem.
> > > >
> > > > So there are linters will fire for this code and say it is not
> > > > pythonic. It is only a linter warning vs asking to support an 8 year
> > > > old out of support distribution. There are other cases, such as
> > > > improving the C code structure, where we've failed to land changes
> > > > because of build errors on old distributions. This could indicate perf
> > > > code is wrong or the distribution is wrong. I'm saying that if we
> > > > believe in the perf code being correct and the distribution is out of
> > > > support, then we should keep the perf code as-is and the issue is one
> > > > for user of the out-of-support distribution.
> > > >
> > > > > Someone reported a problem in a system they used, the author of the code
> > > > > in question posted a patch allowing perf to be used in such old systems,
> > > > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > > > on.
> > >
> > > Considering the proposed patch, can you be sure that replacing the
> > > f-string format with the legacy format won't cause a regression for
> > > some python3 user somewhere when this hits the real world? Even
> > > if it does not cause a regression today, as new versions and features
> > > are added to python3, can you be sure none of those new features
> > > will depend on the upgrade from the legacy format to the f-string
> > > format here to work properly? So many regressions happen because
> > > the people who write patches cannot possibly foresee how their
> > > patch is going to affect the millions of Linux users out there, but still
> > > they are certain it will not cause a regression somewhere. So how
> > > can the chances that this patch will cause a regression be minimized?
> > >
> > > It seems to me for this to be suitable for the Linux kernel, the
> > > default should be to use the modern python3 format and only
> > > enable python2 compatibility via a sysctl setting and/or kernel boot
> > > option for those who are still using python2. There should be no
> > > change to the behavior of the kernel for users who have upgraded
> > > to python3. But I don't see any such consideration for python3
> > > users in this patch.
> >
> > Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
> > does not apply. But can't the script implement this simple logic:
> >
> > If python version = 3 use f-string format
> > if python version = 2 use traditional string format
>
> Doing this in the script would be noisy, having two scripts less than
> ideal. I'd suggest we wait two weeks, declare the official death of
> RHEL7 without "rpm -i python3" and then revert the python3 to python2
> patch. There are plenty of things to worry about and python2 shouldn't
> be one of them (it died over 2 years ago).
I see this has already been committed. I agree it should not
stay in the kernel tree for long. At some point in the future
it will most likely cause problems if it is not reverted.
Chuck
>
> Thanks,
> Ian
>
> > Best regards,
> >
> > Chuck
On Wed, Aug 17, 2022 at 09:12:24PM -0400, Chuck Zmudzinski wrote:
[...]
> > > > > > Someone reported a problem in a system they used, the author of the code
> > > > > > in question posted a patch allowing perf to be used in such old systems,
> > > > > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > > > > on.
> > > >
> > > > Considering the proposed patch, can you be sure that replacing the
> > > > f-string format with the legacy format won't cause a regression for
> > > > some python3 user somewhere when this hits the real world? Even
> > > > if it does not cause a regression today, as new versions and features
> > > > are added to python3, can you be sure none of those new features
> > > > will depend on the upgrade from the legacy format to the f-string
> > > > format here to work properly? So many regressions happen because
> > > > the people who write patches cannot possibly foresee how their
> > > > patch is going to affect the millions of Linux users out there, but still
> > > > they are certain it will not cause a regression somewhere. So how
> > > > can the chances that this patch will cause a regression be minimized?
> > > >
> > > > It seems to me for this to be suitable for the Linux kernel, the
> > > > default should be to use the modern python3 format and only
> > > > enable python2 compatibility via a sysctl setting and/or kernel boot
> > > > option for those who are still using python2. There should be no
> > > > change to the behavior of the kernel for users who have upgraded
> > > > to python3. But I don't see any such consideration for python3
> > > > users in this patch.
> > >
> > > Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
> > > does not apply. But can't the script implement this simple logic:
> > >
> > > If python version = 3 use f-string format
> > > if python version = 2 use traditional string format
> >
> > Doing this in the script would be noisy, having two scripts less than
> > ideal. I'd suggest we wait two weeks, declare the official death of
> > RHEL7 without "rpm -i python3" and then revert the python3 to python2
> > patch. There are plenty of things to worry about and python2 shouldn't
> > be one of them (it died over 2 years ago).
>
> I see this has already been committed. I agree it should not
> stay in the kernel tree for long. At some point in the future
> it will most likely cause problems if it is not reverted.
It is a bit confused me that here actually we are worrying about the
python distro issue, e.g. python2 is obsolete, so it's risky to keep
using python2 in the system, especially if python2 has potential
security issue. So the system (e.g. RHEL7) needs to upgrade its python
distro from python2 to python3.
But this doesn't mean the python script cannot be compatible for both
python2 and python3. I verified this patch, it should can work for
both python2 and python3 on my PC.
Another concern is there have some python scripts in perf, I think most
of them are python2 compatible, should we update all of them to be only
python3 compatible?
Thanks,
Leo
On Wed, Aug 17, 2022 at 8:03 PM Leo Yan <[email protected]> wrote:
>
> On Wed, Aug 17, 2022 at 09:12:24PM -0400, Chuck Zmudzinski wrote:
>
> [...]
>
> > > > > > > Someone reported a problem in a system they used, the author of the code
> > > > > > > in question posted a patch allowing perf to be used in such old systems,
> > > > > > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > > > > > on.
> > > > >
> > > > > Considering the proposed patch, can you be sure that replacing the
> > > > > f-string format with the legacy format won't cause a regression for
> > > > > some python3 user somewhere when this hits the real world? Even
> > > > > if it does not cause a regression today, as new versions and features
> > > > > are added to python3, can you be sure none of those new features
> > > > > will depend on the upgrade from the legacy format to the f-string
> > > > > format here to work properly? So many regressions happen because
> > > > > the people who write patches cannot possibly foresee how their
> > > > > patch is going to affect the millions of Linux users out there, but still
> > > > > they are certain it will not cause a regression somewhere. So how
> > > > > can the chances that this patch will cause a regression be minimized?
> > > > >
> > > > > It seems to me for this to be suitable for the Linux kernel, the
> > > > > default should be to use the modern python3 format and only
> > > > > enable python2 compatibility via a sysctl setting and/or kernel boot
> > > > > option for those who are still using python2. There should be no
> > > > > change to the behavior of the kernel for users who have upgraded
> > > > > to python3. But I don't see any such consideration for python3
> > > > > users in this patch.
> > > >
> > > > Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
> > > > does not apply. But can't the script implement this simple logic:
> > > >
> > > > If python version = 3 use f-string format
> > > > if python version = 2 use traditional string format
> > >
> > > Doing this in the script would be noisy, having two scripts less than
> > > ideal. I'd suggest we wait two weeks, declare the official death of
> > > RHEL7 without "rpm -i python3" and then revert the python3 to python2
> > > patch. There are plenty of things to worry about and python2 shouldn't
> > > be one of them (it died over 2 years ago).
> >
> > I see this has already been committed. I agree it should not
> > stay in the kernel tree for long. At some point in the future
> > it will most likely cause problems if it is not reverted.
>
> It is a bit confused me that here actually we are worrying about the
> python distro issue, e.g. python2 is obsolete, so it's risky to keep
> using python2 in the system, especially if python2 has potential
> security issue. So the system (e.g. RHEL7) needs to upgrade its python
> distro from python2 to python3.
>
> But this doesn't mean the python script cannot be compatible for both
> python2 and python3. I verified this patch, it should can work for
> both python2 and python3 on my PC.
>
> Another concern is there have some python scripts in perf, I think most
> of them are python2 compatible, should we update all of them to be only
> python3 compatible?
I think there are a lot of things that need to be done for python in
the perf build. For example, perf script is using deprecated python C
APIs. . As you say string formatting isn't the biggest issue. What I
think we need to do is set a minimum bar for what is supported, for
jevents.py that is python 3.6.
The problem with python2 compatibility can be seen with this:
https://lore.kernel.org/all/[email protected]/
python3 is deprecating APIs which are the only API choices on python2.
So we can:
1) have 2 scripts, one for python2 and one for python3, possibly
varying by release of python3 depending on when a deprecated API is
removed
2) always be stuck on python 2 as our lowest bar for compatibility
3) forget about python 2, set compatibility at some reasonably but not
totally new version of python like 3.6 and move this forward inline
with supported versions by the python community
I favor (3) not least as testing (1) is a challenge/chore and if
something is wrong with python2, well it is on us. I think we
shouldn't aim to support more python than what the python community
itself aims to support. They are clear that python 2 is dead.
Going back to f-strings, they are considered the more pythonic way to
write things and make it easier to read code, eye-ball mistakes, etc.
We want to have the best code possible in the perf codebase and so
changes to use f-strings to the python scripts in perf should be
welcomed - RHEL7 customers will just have to get with the program imo,
well they should upgrade off of RHEL7. Python 3.6 is hardly new and
this causes issues in jevents.py, for example, as I can't rely on the
string removesuffix function being available (added in python 3.9).
There is a bunch of clean up necessary to get rid of python 2 from the
build, and we took a step in this direction by defaulting to python 3
(not without pain) in:
https://lore.kernel.org/all/[email protected]/
Should we make all the scripts python 3 only? It comes down to people
writing patches. If someone updates a script and uses an f-string, I
don't think we should code review and reject the patch. Should we
proactively go and clean up python code in perf to make it more
pythonic? Sounds good to me, and I might suggest a list of other clean
ups we need to do given you have the time :-)
Thanks,
Ian
> Thanks,
> Leo
On Thu, Aug 18, 2022 at 07:52:22AM -0700, Ian Rogers wrote:
[...]
> > > I see this has already been committed. I agree it should not
> > > stay in the kernel tree for long. At some point in the future
> > > it will most likely cause problems if it is not reverted.
> >
> > It is a bit confused me that here actually we are worrying about the
> > python distro issue, e.g. python2 is obsolete, so it's risky to keep
> > using python2 in the system, especially if python2 has potential
> > security issue. So the system (e.g. RHEL7) needs to upgrade its python
> > distro from python2 to python3.
> >
> > But this doesn't mean the python script cannot be compatible for both
> > python2 and python3. I verified this patch, it should can work for
> > both python2 and python3 on my PC.
> >
> > Another concern is there have some python scripts in perf, I think most
> > of them are python2 compatible, should we update all of them to be only
> > python3 compatible?
>
> I think there are a lot of things that need to be done for python in
> the perf build. For example, perf script is using deprecated python C
> APIs. . As you say string formatting isn't the biggest issue. What I
> think we need to do is set a minimum bar for what is supported, for
> jevents.py that is python 3.6.
>
> The problem with python2 compatibility can be seen with this:
> https://lore.kernel.org/all/[email protected]/
> python3 is deprecating APIs which are the only API choices on python2.
> So we can:
> 1) have 2 scripts, one for python2 and one for python3, possibly
> varying by release of python3 depending on when a deprecated API is
> removed
> 2) always be stuck on python 2 as our lowest bar for compatibility
> 3) forget about python 2, set compatibility at some reasonably but not
> totally new version of python like 3.6 and move this forward inline
> with supported versions by the python community
>
> I favor (3) not least as testing (1) is a challenge/chore and if
> something is wrong with python2, well it is on us. I think we
> shouldn't aim to support more python than what the python community
> itself aims to support. They are clear that python 2 is dead.
I agree that option 3 is the good way to move forward.
Thanks a lot for detailed explaination.
> Going back to f-strings, they are considered the more pythonic way to
> write things and make it easier to read code, eye-ball mistakes, etc.
> We want to have the best code possible in the perf codebase and so
> changes to use f-strings to the python scripts in perf should be
> welcomed - RHEL7 customers will just have to get with the program imo,
> well they should upgrade off of RHEL7. Python 3.6 is hardly new and
> this causes issues in jevents.py, for example, as I can't rely on the
> string removesuffix function being available (added in python 3.9).
>
> There is a bunch of clean up necessary to get rid of python 2 from the
> build, and we took a step in this direction by defaulting to python 3
> (not without pain) in:
> https://lore.kernel.org/all/[email protected]/
>
> Should we make all the scripts python 3 only? It comes down to people
> writing patches. If someone updates a script and uses an f-string, I
> don't think we should code review and reject the patch. Should we
> proactively go and clean up python code in perf to make it more
> pythonic? Sounds good to me, and I might suggest a list of other clean
> ups we need to do given you have the time :-)
It's fine for me to revert this patch in an appropriate time.
Regard to other python scripts in Perf, seems to me a pragmatic method is
that as you said we can set a bar for python version (e.g. 3.6), then we
can go through python scripts one by one to test if any script is not
python 3.6 compatiable or not. If not, then we can commit patch for
it.
"the more pythonic way" is a subjective words? :) I am lack the
knowledge to judge if any things are more "pythonic" and can replace with
the existed code. But I think it would be a good start to use f-string
to replace old print sentences. When I have some free time, I will come
back to check with you and other maintainers if I can help a bit for this.
Thanks,
Leo
On 8/19/2022 2:48 AM, Leo Yan wrote:
> On Thu, Aug 18, 2022 at 07:52:22AM -0700, Ian Rogers wrote:
>
> [...]
>
> > > > I see this has already been committed. I agree it should not
> > > > stay in the kernel tree for long. At some point in the future
> > > > it will most likely cause problems if it is not reverted.
> > >
>
>
> It's fine for me to revert this patch in an appropriate time.
An appropriate time presumably means when there is no more significant
demand for mainline kernel support on older distros like RHEL 7 and clones
that are still using python2. I also presume you plan to post the proposed
revert to the appropriate mailing lists before committing it, giving time for
anyone who wants to keep python2 compatibility a chance to make their
case.
Thanks,
Chuck