Re: [PATCH] audit: remove the audit freelist

[William Roberts <bill.c.roberts@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4742 bytes --]

On Nov 29, 2016 07:10, "Florian Westphal" <fw@strlen.de> wrote:
>
> allows better debugging as freeing audit buffers now always honors slub
> debug hooks (e.g. object poisoning) and leak checker can detect the
> free operation.
>
> Removal also results in a small speedup (using
> single rule 'iptables -A INPUT -i lo -j AUDIT --type drop'):
>
> super_netperf 4 -H 127.0.0.1 -l 360 -t UDP_RR -- -R 1 -m 64
> Before:
> 294953
> After:
> 298013

A bigger number is better? Sorry for the html Gmail app on Android won't
let me switch.

>
> (alloc/free no longer serializes on spinlock, allocator can use percpu
>  pool).
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  kernel/audit.c | 53 ++++++++---------------------------------------------
>  1 file changed, 8 insertions(+), 45 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f1ca11613379..396868dc523a 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -131,13 +131,6 @@ static int audit_net_id;
>  /* Hash for inode-based rules */
>  struct list_head audit_inode_hash[AUDIT_INODE_BUCKETS];
>
> -/* The audit_freelist is a list of pre-allocated audit buffers (if more
> - * than AUDIT_MAXFREE are in use, the audit buffer is freed instead of
> - * being placed on the freelist). */
> -static DEFINE_SPINLOCK(audit_freelist_lock);
> -static int        audit_freelist_count;
> -static LIST_HEAD(audit_freelist);
> -
>  static struct sk_buff_head audit_skb_queue;
>  /* queue of skbs to send to auditd when/if it comes back */
>  static struct sk_buff_head audit_skb_hold_queue;
> @@ -164,17 +157,11 @@ DEFINE_MUTEX(audit_cmd_mutex);
>   * should be at least that large. */
>  #define AUDIT_BUFSIZ 1024
>
> -/* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
> - * audit_freelist.  Doing so eliminates many kmalloc/kfree calls. */
> -#define AUDIT_MAXFREE  (2*NR_CPUS)
> -
> -/* The audit_buffer is used when formatting an audit record.  The caller
> - * locks briefly to get the record off the freelist or to allocate the
> - * buffer, and locks briefly to send the buffer to the netlink layer or
> +/* The audit_buffer is used when formatting an audit record.
> + * The caller locks briefly to send the buffer to the netlink layer or
>   * to place it on a transmit queue.  Multiple audit_buffers can be in
>   * use simultaneously. */
>  struct audit_buffer {
> -       struct list_head     list;
>         struct sk_buff       *skb;      /* formatted skb ready to send */
>         struct audit_context *ctx;      /* NULL or associated context */
>         gfp_t                gfp_mask;
> @@ -1247,43 +1234,22 @@ __setup("audit_backlog_limit=",
audit_backlog_limit_set);
>
>  static void audit_buffer_free(struct audit_buffer *ab)
>  {
> -       unsigned long flags;
> -
>         if (!ab)
>                 return;
>
>         kfree_skb(ab->skb);
> -       spin_lock_irqsave(&audit_freelist_lock, flags);
> -       if (audit_freelist_count > AUDIT_MAXFREE)
> -               kfree(ab);
> -       else {
> -               audit_freelist_count++;
> -               list_add(&ab->list, &audit_freelist);
> -       }
> -       spin_unlock_irqrestore(&audit_freelist_lock, flags);
> +       kfree(ab);
>  }
>
>  static struct audit_buffer * audit_buffer_alloc(struct audit_context
*ctx,
>                                                 gfp_t gfp_mask, int type)
>  {
> -       unsigned long flags;
> -       struct audit_buffer *ab = NULL;
> +       struct audit_buffer *ab;
>         struct nlmsghdr *nlh;
>
> -       spin_lock_irqsave(&audit_freelist_lock, flags);
> -       if (!list_empty(&audit_freelist)) {
> -               ab = list_entry(audit_freelist.next,
> -                               struct audit_buffer, list);
> -               list_del(&ab->list);
> -               --audit_freelist_count;
> -       }
> -       spin_unlock_irqrestore(&audit_freelist_lock, flags);
> -
> -       if (!ab) {
> -               ab = kmalloc(sizeof(*ab), gfp_mask);
> -               if (!ab)
> -                       goto err;
> -       }
> +       ab = kmalloc(sizeof(*ab), gfp_mask);
> +       if (!ab)
> +               return NULL;
>
>         ab->ctx = ctx;
>         ab->gfp_mask = gfp_mask;
> @@ -1294,13 +1260,10 @@ static struct audit_buffer *
audit_buffer_alloc(struct audit_context *ctx,
>
>         nlh = nlmsg_put(ab->skb, 0, 0, type, 0, 0);
>         if (!nlh)
> -               goto out_kfree_skb;
> +               goto err;
>
>         return ab;
>
> -out_kfree_skb:
> -       kfree_skb(ab->skb);
> -       ab->skb = NULL;
>  err:
>         audit_buffer_free(ab);
>         return NULL;
> --
> 2.7.3
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Type: text/html, Size: 6473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]