* [PATCH] audit: we don't need to __set_current_state(TASK_RUNNING) @ 2016-04-05 18:31 Paul Moore 2016-04-06 16:56 ` Richard Guy Briggs 0 siblings, 1 reply; 3+ messages in thread From: Paul Moore @ 2016-04-05 18:31 UTC (permalink / raw) To: linux-audit From: Paul Moore <paul@paul-moore.com> Remove the calls to __set_current_state() to mark the task as running and do some related cleanup in wait_for_auditd() to limit the amount of work we do when we aren't going to reschedule the current task. Signed-off-by: Paul Moore <paul@paul-moore.com> --- kernel/audit.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 3a3e5de..f52fbef 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -430,7 +430,6 @@ restart: attempts, audit_pid); set_current_state(TASK_INTERRUPTIBLE); schedule(); - __set_current_state(TASK_RUNNING); goto restart; } } @@ -1324,15 +1323,14 @@ static inline void audit_get_stamp(struct audit_context *ctx, static long wait_for_auditd(long sleep_time) { DECLARE_WAITQUEUE(wait, current); - set_current_state(TASK_UNINTERRUPTIBLE); - add_wait_queue_exclusive(&audit_backlog_wait, &wait); if (audit_backlog_limit && - skb_queue_len(&audit_skb_queue) > audit_backlog_limit) + skb_queue_len(&audit_skb_queue) > audit_backlog_limit) { + add_wait_queue_exclusive(&audit_backlog_wait, &wait); + set_current_state(TASK_UNINTERRUPTIBLE); sleep_time = schedule_timeout(sleep_time); - - __set_current_state(TASK_RUNNING); - remove_wait_queue(&audit_backlog_wait, &wait); + remove_wait_queue(&audit_backlog_wait, &wait); + } return sleep_time; } ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] audit: we don't need to __set_current_state(TASK_RUNNING) 2016-04-05 18:31 [PATCH] audit: we don't need to __set_current_state(TASK_RUNNING) Paul Moore @ 2016-04-06 16:56 ` Richard Guy Briggs 2016-04-06 17:41 ` Paul Moore 0 siblings, 1 reply; 3+ messages in thread From: Richard Guy Briggs @ 2016-04-06 16:56 UTC (permalink / raw) To: Paul Moore; +Cc: linux-audit On 16/04/05, Paul Moore wrote: > From: Paul Moore <paul@paul-moore.com> > > Remove the calls to __set_current_state() to mark the task as running > and do some related cleanup in wait_for_auditd() to limit the amount > of work we do when we aren't going to reschedule the current task. Wow, the logic of this goes back to 9ad9ad38 (2005-06-22, AUDIT: Wait for backlog to clear when generating messages.) > Signed-off-by: Paul Moore <paul@paul-moore.com> These changes look good to me. > --- > kernel/audit.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 3a3e5de..f52fbef 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -430,7 +430,6 @@ restart: > attempts, audit_pid); > set_current_state(TASK_INTERRUPTIBLE); > schedule(); > - __set_current_state(TASK_RUNNING); > goto restart; > } > } > @@ -1324,15 +1323,14 @@ static inline void audit_get_stamp(struct audit_context *ctx, > static long wait_for_auditd(long sleep_time) > { > DECLARE_WAITQUEUE(wait, current); > - set_current_state(TASK_UNINTERRUPTIBLE); > - add_wait_queue_exclusive(&audit_backlog_wait, &wait); > > if (audit_backlog_limit && > - skb_queue_len(&audit_skb_queue) > audit_backlog_limit) > + skb_queue_len(&audit_skb_queue) > audit_backlog_limit) { > + add_wait_queue_exclusive(&audit_backlog_wait, &wait); > + set_current_state(TASK_UNINTERRUPTIBLE); > sleep_time = schedule_timeout(sleep_time); > - > - __set_current_state(TASK_RUNNING); > - remove_wait_queue(&audit_backlog_wait, &wait); > + remove_wait_queue(&audit_backlog_wait, &wait); > + } > > return sleep_time; > } - RGB -- Richard Guy Briggs <rbriggs@redhat.com> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] audit: we don't need to __set_current_state(TASK_RUNNING) 2016-04-06 16:56 ` Richard Guy Briggs @ 2016-04-06 17:41 ` Paul Moore 0 siblings, 0 replies; 3+ messages in thread From: Paul Moore @ 2016-04-06 17:41 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: linux-audit On Wednesday, April 06, 2016 12:56:24 PM Richard Guy Briggs wrote: > On 16/04/05, Paul Moore wrote: > > From: Paul Moore <paul@paul-moore.com> > > > > Remove the calls to __set_current_state() to mark the task as running > > and do some related cleanup in wait_for_auditd() to limit the amount > > of work we do when we aren't going to reschedule the current task. > > Wow, the logic of this goes back to 9ad9ad38 (2005-06-22, AUDIT: Wait > for backlog to clear when generating messages.) Yeah, I didn't think much of it until Jiri submitted that last patch and figured I should check how we were doing handling the rescheduling in the rest of the code. I think it stayed the way it was for so long because technically it wasn't hurting anything. > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > These changes look good to me. > > > --- > > > > kernel/audit.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 3a3e5de..f52fbef 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > > > @@ -430,7 +430,6 @@ restart: > > attempts, audit_pid); > > > > set_current_state(TASK_INTERRUPTIBLE); > > schedule(); > > > > - __set_current_state(TASK_RUNNING); > > > > goto restart; > > > > } > > > > } > > > > @@ -1324,15 +1323,14 @@ static inline void audit_get_stamp(struct > > audit_context *ctx,> > > static long wait_for_auditd(long sleep_time) > > { > > > > DECLARE_WAITQUEUE(wait, current); > > > > - set_current_state(TASK_UNINTERRUPTIBLE); > > - add_wait_queue_exclusive(&audit_backlog_wait, &wait); > > > > if (audit_backlog_limit && > > > > - skb_queue_len(&audit_skb_queue) > audit_backlog_limit) > > + skb_queue_len(&audit_skb_queue) > audit_backlog_limit) { > > + add_wait_queue_exclusive(&audit_backlog_wait, &wait); > > + set_current_state(TASK_UNINTERRUPTIBLE); > > > > sleep_time = schedule_timeout(sleep_time); > > > > - > > - __set_current_state(TASK_RUNNING); > > - remove_wait_queue(&audit_backlog_wait, &wait); > > + remove_wait_queue(&audit_backlog_wait, &wait); > > + } > > > > return sleep_time; > > > > } > > - RGB > > -- > Richard Guy Briggs <rbriggs@redhat.com> > Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, > Red Hat Remote, Ottawa, Canada > Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 -- paul moore security @ redhat ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-04-06 17:41 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-05 18:31 [PATCH] audit: we don't need to __set_current_state(TASK_RUNNING) Paul Moore 2016-04-06 16:56 ` Richard Guy Briggs 2016-04-06 17:41 ` Paul Moore
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox