Opinion please

[Timothy R. Chavez]

Hello,

Serge pointed out something to me on an internal list here and I wanted to 
poll for opinions.  Which would you prefer?  I think adding another parameter 
to audit_send_reply to specify GFP_ATOMIC/KERNEL would be simplest.  But is 
it wise to do so?

Here's what he had to say:

Hey Tim,

it looks like w_master is now adequately protected by the spinlock.

Only problem is that you might sleep under rcu_read_lock().  In
audit_list_watches(), you call audit_send_watch() from under
rcu_read_lock.  audit_send_watch() calls audit_to_transport, which calls
kmalloc(GFP_KERNEL).  Switch that to GFP_ATOMIC.

audit_send_watch() also calls audit_send_reply(), which calls
alloc_skb(GFP_KERNEL), and netlink_unicast.  You could patch
audit_send_reply() to take a gfp_mask argument, and send
gfp_mask=GFP_ATOMIC from audit_send_watch().

But I'm not sure whether netlink_unicast is a problem (sure looks like
it - it puts current on a waitqueue for one thing) or what you should do
about it.  You might just need to build a list of things to send under
the rcu_read_lock(), then send each element once you rcu_read_unlock().
So something like:

struct audit_skb_list {
        struct hlist_node list;
        void *memblk;
        size_t size;
};

audit_list_watches(int pid, int seq)
{
        struct hlist_head skb_list;
        struct audit_skb_list listentry;
        struct hlist_node *tmp, *pos;

        INIT_HLIST_HEAD(&skb_list);

        rcu_read_lock();
        hlist_for_each_entry_rcu(...) {
                tmp = audit_build_watch(pid, seq, wentry->w_watch);
                if (tmp)
                        hlist_add_head(tmp, &skb_list);
        }
        rcu_read_unlock();

        hlist_for_each_entry_safe(listentry, pos, tmp, &skb_list, list) {
                audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 0, 1,
                        listentry->memblk, listentry->size);
                kfree(listentry);
        }

        audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 1, 1, NULL, 0);

} 


-- 
-tim