| Message ID | 20240307133916.3782068-4-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/mm: LAM fixups and cleanups | expand |
On Thu, Mar 07, 2024 at 01:39:16PM +0000, Yosry Ahmed wrote: > In prctl_enable_tagged_addr(), we check that nr_bits is in the correct > range, but we do so in a twisted if/else block where the correct case is > sandwiched between two error cases doing exactly the same thing. > > Simplify the if condition and pull the correct case outside with the > rest of the success code path. I'm okay either way. I structured the code this way as I had separate patch that adds also LAM_U48. But it is unlikely to get upstreamed.
On Thu, Mar 07, 2024 at 07:31:44PM +0200, Kirill A. Shutemov wrote: > On Thu, Mar 07, 2024 at 01:39:16PM +0000, Yosry Ahmed wrote: > > In prctl_enable_tagged_addr(), we check that nr_bits is in the correct > > range, but we do so in a twisted if/else block where the correct case is > > sandwiched between two error cases doing exactly the same thing. > > > > Simplify the if condition and pull the correct case outside with the > > rest of the success code path. > > I'm okay either way. > > I structured the code this way as I had separate patch that adds also > LAM_U48. But it is unlikely to get upstreamed. I see, thanks for the context. For now, I think this makes the code a little bit clearer.
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 33b268747bb7b..3f381906bbe1d 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -771,17 +771,13 @@ static int prctl_enable_tagged_addr(struct mm_struct *mm, unsigned long nr_bits) return -EBUSY; } - if (!nr_bits) { - mmap_write_unlock(mm); - return -EINVAL; - } else if (nr_bits <= LAM_U57_BITS) { - mm->context.lam_cr3_mask = X86_CR3_LAM_U57; - mm->context.untag_mask = ~GENMASK(62, 57); - } else { + if (!nr_bits || nr_bits > LAM_U57_BITS) { mmap_write_unlock(mm); return -EINVAL; } + mm->context.lam_cr3_mask = X86_CR3_LAM_U57; + mm->context.untag_mask = ~GENMASK(62, 57); write_cr3(__read_cr3() | mm->context.lam_cr3_mask); set_tlbstate_lam_mode(mm); set_bit(MM_CONTEXT_LOCK_LAM, &mm->context.flags);
In prctl_enable_tagged_addr(), we check that nr_bits is in the correct range, but we do so in a twisted if/else block where the correct case is sandwiched between two error cases doing exactly the same thing. Simplify the if condition and pull the correct case outside with the rest of the success code path. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- arch/x86/kernel/process_64.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)