On Mon, Jun 7, 2021 at 2:40 PM Richard Guy Briggs <[email protected]> wrote: > On 2021-06-05 23:23, Paul Moore wrote: > > [NOTE: As this is an RFC patch, I wanted to add some commentary at > > the top of the patch description explaining where this patch came > > from and what testing has been done. This patch is a derivative > > of another unreleased patch that removed all of the wake up calls > > from the audit_log_start() and audit_log_end() functions; while > > that patch worked well, there we some record losees with high > > volume burst traffic in the case of `auditctl -a task,never` or > > CONFIG_AUDITSYSCALL=n. As this patch doesn't completely remove > > the wake up calls these two cases should behave similarly to how > > they do today. As far as testing is concerned, this patch passes > > both the audit-testsuite and selinux-testsuite without problem and > > with expected audit queue losses (no losses outside of the tests > > which attempt to generate losses). This patch also preserves the > > ability for the system to continue to function, in the > > `auditctl -a exit,always -S all` case, albeit very slowly.] > > > > When audit is enabled, the kauditd_thread() function runs in its own > > kernel thread, emptying the audit record queue and sending the > > entries to userspace via different means. As part of its normal > > processing it goes to sleep after emptying the queue and waits for > > the audit functions to wake it. > > > > The current audit kernel code attempts to wake the kauditd thread > > after each entry is added to the queue. Considering that a single > > syscall can have multiple audit records, with each wake attempt > > involving locking and additional processing, this can be rather > > wasteful. In an effort to limit the number of wake attempts without > > unnecessarily risking the audit queue size this patch does the > > following: > > > > * In the case of syscall auditing the wake up call is moved from > > audit_log_end() to audit_log_exit() meaning that the kauditd > > thread is only woken once, at the end of the syscall, after all of > > the syscall's audit records have been added to the queue. > > > > * In the case where audit records are generated outside of a syscall > > context, the wake up call in audit_log_end() is preserved in order > > to ensure that these records do not suffer any additional latency > > or put unnecessary pressure on the queue. This is done through > > some additional checking in audit_log_start() and an additional > > flag in the audit_buffer struct. > > > > * The audit_log_start() function never attempts to wake the kauditd > > thread. In the current code this is only done when the queue is > > under pressure, but at that point it is extremely likely that the > > thread is already active or scheduled, do we should be able to > > safely remove this wake attempt. > > > > * Always wake the kauditd thread after processing management and > > user records in audit_receive_msg(). This should be relatively > > low frequency compared to the other audit sources, and doing a > > wake call here helps keep record latency low and the queue size > > in check. > > > > * The kauditd thread itself is now a bit better at handling high > > volume audit record bursts. Previously after emptying the queue > > the thread would wake every process that was blocked and then go > > to sleep; possibly going to sleep just a flood of new records > > are added to the queue. The new kauditd thread approach wakes all > > of the blocked processes and then reschedules itself in > > anticipation of new records. When the thread returns to execution > > it checks the queue and if there are any records present it > > immediately starts processing them, if the queue is empty the > > kauditd thread goes back to sleep. > > > > Signed-off-by: Paul Moore <[email protected]> > > This looks good to me. Thank you for the thorough description. I can > see how this work was provoked given some of the other work in progress > and some of the minor changes that will be needed to those other works > as a result. > > Acked-by: Richard Guy Briggs <[email protected]>
Thanks for taking a look. As a FYI, I'm going to sit on this until the io_uring patches are merged as there is some overlap, and I want to possibly play with this a bit more as well. I wanted to post this to see if people had any strong feelings about this, and of course just to get it "out there" so I wouldn't forget about it :) -- paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://listman.redhat.com/mailman/listinfo/linux-audit
