| Message ID | 20240805173816.3722002-1-jesse@rivosinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] RISC-V: Add parameter to unaligned access speed | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote: > > Add a kernel parameter to the unaligned access speed. This allows > skiping of the speed tests for unaligned accesses, which often is very > slow. > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> How come this is a command line parameter rather than a Kconfig option? I could be wrong, so I'll lay out my rationale and people can pick it apart if I've got a bad assumption. I think of commandline parameters as (mostly) something end users twiddle with, versus kconfig options as something system builders set up. I'd largely expect end users not to notice two ticks at boot time. I'd expect its system builders and fleet managers, who know their hardware and build their kernels optimized for it, are the ones who would want to shave off this time and go straight to the known answer. Anecdotally, at ChromeOS we had a strong preference for Kconfig options, as they were easier to compose and maintain than a loose pile of commandline arguments. The commit text doesn't go into the rationale, intended audience, or expected usage, so maybe my guesses miss the mark on what you're thinking. -Evan
On Mon, Aug 05, 2024 at 11:38:23AM -0700, Evan Green wrote: > On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote: > > > > Add a kernel parameter to the unaligned access speed. This allows > > skiping of the speed tests for unaligned accesses, which often is very > > slow. > > > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> > > How come this is a command line parameter rather than a Kconfig > option? I could be wrong, so I'll lay out my rationale and people can > pick it apart if I've got a bad assumption. > > I think of commandline parameters as (mostly) something end users > twiddle with, versus kconfig options as something system builders set > up. I'd largely expect end users not to notice two ticks at boot time. > I'd expect its system builders and fleet managers, who know their > hardware and build their kernels optimized for it, are the ones who > would want to shave off this time and go straight to the known answer. > Anecdotally, at ChromeOS we had a strong preference for Kconfig > options, as they were easier to compose and maintain than a loose pile > of commandline arguments. > > The commit text doesn't go into the rationale, intended audience, or > expected usage, so maybe my guesses miss the mark on what you're > thinking. > -Evan There was a brief discussion about this on Jesse's series about vector unaligned support [1]. The original idea was to use Zicclsm to allow people to set the unaligned access speed on pre-compiled distro kernels. However Zicclsm isn't useful so the alternative route was to use a kernel arg. There is already support for a Kconfig, the kernel arg is just another option for users. Link: https://lore.kernel.org/lkml/af3152b6-adf7-40fa-b2a1-87e66eec45b0@rivosinc.com/ [1] - Charlie
On Mon, Aug 5, 2024 at 11:48 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > On Mon, Aug 05, 2024 at 11:38:23AM -0700, Evan Green wrote: > > On Mon, Aug 5, 2024 at 10:38 AM Jesse Taube <jesse@rivosinc.com> wrote: > > > > > > Add a kernel parameter to the unaligned access speed. This allows > > > skiping of the speed tests for unaligned accesses, which often is very > > > slow. > > > > > > Signed-off-by: Jesse Taube <jesse@rivosinc.com> > > > > How come this is a command line parameter rather than a Kconfig > > option? I could be wrong, so I'll lay out my rationale and people can > > pick it apart if I've got a bad assumption. > > > > I think of commandline parameters as (mostly) something end users > > twiddle with, versus kconfig options as something system builders set > > up. I'd largely expect end users not to notice two ticks at boot time. > > I'd expect its system builders and fleet managers, who know their > > hardware and build their kernels optimized for it, are the ones who > > would want to shave off this time and go straight to the known answer. > > Anecdotally, at ChromeOS we had a strong preference for Kconfig > > options, as they were easier to compose and maintain than a loose pile > > of commandline arguments. > > > > The commit text doesn't go into the rationale, intended audience, or > > expected usage, so maybe my guesses miss the mark on what you're > > thinking. > > -Evan > > There was a brief discussion about this on Jesse's series about vector > unaligned support [1]. The original idea was to use Zicclsm to allow > people to set the unaligned access speed on pre-compiled distro kernels. > However Zicclsm isn't useful so the alternative route was to use a > kernel arg. There is already support for a Kconfig, the kernel arg is > just another option for users. > > Link: > https://lore.kernel.org/lkml/af3152b6-adf7-40fa-b2a1-87e66eec45b0@rivosinc.com/ > [1] Ah got it, thanks for the explanation Charlie! If there are consumers for this then the concept seems fine with me. -Evan
diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c index 1548eb10ae4f..02f7a92a5fa0 100644 --- a/arch/riscv/kernel/unaligned_access_speed.c +++ b/arch/riscv/kernel/unaligned_access_speed.c @@ -400,13 +400,94 @@ static int vec_check_unaligned_access_speed_all_cpus(void *unused __always_unuse } #endif +static DEFINE_PER_CPU(long, unaligned_scalar_speed_param) = RISCV_HWPROBE_MISALIGNED_UNKNOWN; + +static int __init set_unaligned_scalar_speed_param(char *str) +{ + cpumask_var_t mask; + int ret, cpu; + long speed = RISCV_HWPROBE_MISALIGNED_UNKNOWN; + + if (!strncmp(str, "fast,", 5)) { + str += 5; + speed = RISCV_HWPROBE_MISALIGNED_FAST; + } + + if (!strncmp(str, "slow,", 5)) { + str += 5; + speed = RISCV_HWPROBE_MISALIGNED_SLOW; + } + if (speed == RISCV_HWPROBE_MISALIGNED_UNKNOWN) { + pr_warn("Invalid unaligned access speed parameter\n"); + return 1; + } + + if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) + return -ENOMEM; + + ret = cpulist_parse(str, mask); + + for_each_cpu(cpu, mask) + if (per_cpu(unaligned_scalar_speed_param, cpu) == RISCV_HWPROBE_MISALIGNED_UNKNOWN) + per_cpu(unaligned_scalar_speed_param, cpu) = speed; + + free_cpumask_var(mask); + return ret == 0; +} +__setup("unaligned_scalar_speed=", set_unaligned_scalar_speed_param); + +static DEFINE_PER_CPU(long, unaligned_vector_speed_param) = RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN; + +static int __init set_unaligned_vector_speed_param(char *str) +{ + cpumask_var_t mask; + int ret, cpu; + long speed = RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN; + + if (!strncmp(str, "fast,", 5)) { + str += 5; + speed = RISCV_HWPROBE_VECTOR_MISALIGNED_FAST; + } + + if (!strncmp(str, "slow,", 5)) { + str += 5; + speed = RISCV_HWPROBE_VECTOR_MISALIGNED_SLOW; + } + if (speed == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN) { + pr_warn("Invalid unaligned access speed parameter\n"); + return 1; + } + + if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) + return -ENOMEM; + + ret = cpulist_parse(str, mask); + + for_each_cpu(cpu, mask) + if (per_cpu(unaligned_vector_speed_param, cpu) == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN) + per_cpu(unaligned_vector_speed_param, cpu) = speed; + + free_cpumask_var(mask); + return ret == 0; +} +__setup("unaligned_vector_speed=", set_unaligned_vector_speed_param); + static int check_unaligned_access_all_cpus(void) { + int cpu; bool all_cpus_emulated, all_cpus_vec_unsupported; all_cpus_emulated = check_unaligned_access_emulated_all_cpus(); all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus(); + for_each_online_cpu(cpu) { + if (per_cpu(misaligned_access_speed, cpu) == RISCV_HWPROBE_MISALIGNED_UNKNOWN) + per_cpu(misaligned_access_speed, cpu) = per_cpu(unaligned_scalar_speed_param, cpu); + + if (per_cpu(vector_misaligned_access, cpu) == RISCV_HWPROBE_VECTOR_MISALIGNED_UNKNOWN) + per_cpu(vector_misaligned_access, cpu) = per_cpu(unaligned_vector_speed_param, cpu); + } + pr_info("\e[31m%s vector unaligned access\e[0m\n", all_cpus_vec_unsupported ? "All CPUs do not support" : "At least one cpu supports"); if (!all_cpus_vec_unsupported &&
Add a kernel parameter to the unaligned access speed. This allows skiping of the speed tests for unaligned accesses, which often is very slow. Signed-off-by: Jesse Taube <jesse@rivosinc.com> --- arch/riscv/kernel/unaligned_access_speed.c | 81 ++++++++++++++++++++++ 1 file changed, 81 insertions(+)