Thomas Graf wrote:
>> @@ -150,9 +176,9 @@ int genl_register_ops(struct genl_family *family, struct
>> genl_ops *ops)
>> if (ops->policy)
>> ops->flags |= GENL_CMD_CAP_HASPOL;
>>
>> - genl_lock();
>> + genl_fam_lock(family);
>> list_add_tail(&ops->ops_list, &family->ops_list);
>> - genl_unlock();
>> + genl_fam_unlock(family);
>
> For registering operations, it is sufficient to just acquire the
> family lock, the family itself can't disappear while holding it.
I agree.
>> @@ -303,38 +332,57 @@ static int genl_rcv_msg(struct sk_buff *skb, struct
>> nlmsghdr *nlh)
>> struct genlmsghdr *hdr = nlmsg_data(nlh);
>> int hdrlen, err;
>>
>> + genl_fam_lock(NULL);
>> family = genl_family_find_byid(nlh->nlmsg_type);
>> - if (family == NULL)
>> + if (family == NULL) {
>> + genl_fam_unlock(NULL);
>> return -ENOENT;
>> + }
>> +
>> + /* get particular family lock, but release global family lock
>> + * so registering operations for other families are possible */
>> + genl_onefam_lock(family);
>> + genl_fam_unlock(NULL);
>
> I don't like having two locks for something as trivial as this.
> Basically the only reason the global lock is required here is to
> protect from family removal which can be avoided otherwise by
> using RCU list operations.
>
> Therefore, I'd propose the following lock semantics:
> Use own global mutex to protect writing to the family list, make
> reading side lockless using rcu for use when looking up family
> upon mesage processing. Use a family lock to protect writing to
> operations list and serialize messae processing with unregister
> operations.
I was not aware of RCU lists, but after looking at them, I consider your
proposal to be better. I guess, you would rather write the patch
yourself, so I will wait.
Thanks for help,
Richard
-
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
