lkml.org 
[lkml]   [2023]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH -next] RFC: apparmor: Optimize retrieving current task secid
From
On 8/31/23 19:45, Vinicius Costa Gomes wrote:
> John Johansen <john.johansen@canonical.com> writes:
>
>> On 8/31/23 16:22, Vinicius Costa Gomes wrote:
>>> When running will-it-scale[1] open2_process testcase, in a system with a
>>> large number of cores, a bottleneck in retrieving the current task
>>> secid was detected:
>>>
>>> 27.73% ima_file_check;do_open (inlined);path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_x64 (inlined);do_syscall_64;entry_SYSCALL_64_after_hwframe (inlined);__libc_open64 (inlined)
>>> 27.72% 0.01% [kernel.vmlinux] [k] security_current_getsecid_subj - -
>>> 27.71% security_current_getsecid_subj;ima_file_check;do_open (inlined);path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_x64 (inlined);do_syscall_64;entry_SYSCALL_64_after_hwframe (inlined);__libc_open64 (inlined)
>>> 27.71% 27.68% [kernel.vmlinux] [k] apparmor_current_getsecid_subj - -
>>> 19.94% __refcount_add (inlined);__refcount_inc (inlined);refcount_inc (inlined);kref_get (inlined);aa_get_label (inlined);aa_get_label (inlined);aa_get_current_label (inlined);apparmor_current_getsecid_subj;security_current_getsecid_subj;ima_file_check;do_open (inlined);path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_x64 (inlined);do_syscall_64;entry_SYSCALL_64_after_hwframe (inlined);__libc_open64 (inlined)
>>> 7.72% __refcount_sub_and_test (inlined);__refcount_dec_and_test (inlined);refcount_dec_and_test (inlined);kref_put (inlined);aa_put_label (inlined);aa_put_label (inlined);apparmor_current_getsecid_subj;security_current_getsecid_subj;ima_file_check;do_open (inlined);path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_x64 (inlined);do_syscall_64;entry_SYSCALL_64_after_hwframe (inlined);__libc_open64 (inlined)
>>>
>>> A large amount of time was spent in the refcount.
>>>
>>> The most common case is that the current task label is available, and
>>> no need to take references for that one. That is exactly what the
>>> critical section helpers do, make use of them.
>>>
>>> New perf output:
>>>
>>> 39.12% vfs_open;path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_64;entry_SYSCALL_64_after_hwframe;__libc_open64 (inlined)
>>> 39.07% 0.13% [kernel.vmlinux] [k] do_dentry_open - -
>>> 39.05% do_dentry_open;vfs_open;path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_64;entry_SYSCALL_64_after_hwframe;__libc_open64 (inlined)
>>> 38.71% 0.01% [kernel.vmlinux] [k] security_file_open - -
>>> 38.70% security_file_open;do_dentry_open;vfs_open;path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_64;entry_SYSCALL_64_after_hwframe;__libc_open64 (inlined)
>>> 38.65% 38.60% [kernel.vmlinux] [k] apparmor_file_open - -
>>> 38.65% apparmor_file_open;security_file_open;do_dentry_open;vfs_open;path_openat;do_filp_open;do_sys_openat2;__x64_sys_openat;do_syscall_64;entry_SYSCALL_64_after_hwframe;__libc_open64 (inlined)
>>>
>>> The result is a throughput improvement of around 20% across the board
>>> on the open2 testcase. On more realistic workloads the impact should
>>> be much less.
>>> hrmmm, interesting. its a nice improvement
>>
>>> [1] https://github.com/antonblanchard/will-it-scale
>>>
>>> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
>>
>> Acked-by: John Johansen <john.johansen@canonical.com>
>>
>> do you want me to pull this into apparmor-next or do you have another
>> tree in mind
>>
>
> -next is fine.
>
>>> ---
>>> Sending as RFC because I am not sure there's anything I am missing. My
>>> read of the code tells me it should be fine.
>>
>> it is
>>
>
> Great. Do you want me to send a non-RFC version?
>
you can if you want but there is no need, I can do that small edit

>>>
>>> security/apparmor/lsm.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>>> index 108eccc5ada5..98e65c44ddd5 100644
>>> --- a/security/apparmor/lsm.c
>>> +++ b/security/apparmor/lsm.c
>>> @@ -766,9 +766,9 @@ static void apparmor_bprm_committed_creds(struct linux_binprm *bprm)
>>>
>>> static void apparmor_current_getsecid_subj(u32 *secid)
>>> {
>>> - struct aa_label *label = aa_get_current_label();
>>> + struct aa_label *label = __begin_current_label_crit_section();
>>> *secid = label->secid;
>>> - aa_put_label(label);
>>> + __end_current_label_crit_section(label);
>>> }
>>>
>>> static void apparmor_task_getsecid_obj(struct task_struct *p, u32 *secid)
>>
>
> Cheers,

\
 
 \ /
  Last update: 2023-09-01 04:55    [W:0.037 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site