Baruch Even wrote: > Patrick McHardy wrote: > >>New version of the netlink_has_listeners() patch. >> >>Changes: >> >>- Fix missing listeners bitmap update when there was no delta in the >> number of subscribed groups >>- Use RCU to protect nltable listeners bitmap >> >> >> >>------------------------------------------------------------------------ >> >>[NETLINK]: Add netlink_has_listeners() for checking for multicast listeners >> >>netlink_has_listeners() should be used to avoid unneccessary event message >>generation if there are no listeners. >> > > ... > >> if (nlk->flags & NETLINK_KERNEL_SOCKET) { >>- netlink_table_grab(); >>+ unsigned long *listeners; >>+ >>+ listeners = nl_table[sk->sk_protocol].listeners; >>+ nl_table[sk->sk_protocol].listeners = NULL; >>+ synchronize_rcu(); >>+ kfree(nl_table[sk->sk_protocol].listeners); > > > Doesn't the NULL assignment needs to use rcu_assign_pointer()?
Thinking about it .. using RCU seems entirely unneccessary, assuming callers don't close the kernel socket and call netlink_has_listeners() afterwards, the pointer is always valid. > And isn't the kfree should be on the local listeners variable as opposed > to the "just" NULLed variable? Thanks for catching this. I'm not sure how this slipped in, reverting the patch using cg-admin-uncommit without editing it before shows the correct line, kfree(listeners). - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html