Re: [PATCH v3] rfs: Receive Flow Steering

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 08 avril 2010 à 23:33 -0700, Tom Herbert a écrit :

>  #ifdef CONFIG_RPS
> +/* One global table that all flow-based protocols share. */
> +struct rps_sock_flow_table *rps_sock_flow_table;
> +EXPORT_SYMBOL(rps_sock_flow_table);
> +
> +int rps_sock_flow_sysctl(ctl_table *table, int write, void __user *buffer,
> +			 size_t *lenp, loff_t *ppos)
> +{
> +	unsigned int orig_size, size;
> +	int ret, i;
> +	ctl_table tmp = {
> +		.data = &size,
> +		.maxlen = sizeof(size),
> +		.mode = table->mode
> +	};
> +	struct rps_sock_flow_table *orig_sock_table, *sock_table;
> +
> +	rcu_read_lock();
> +
> +	orig_sock_table = rcu_dereference(rps_sock_flow_table);
> +	size = orig_size = orig_sock_table ? orig_sock_table->mask + 1 : 0;
> +
> +	ret = proc_dointvec(&tmp, write, buffer, lenp, ppos);
> +
> +	if (write) {
> +		if (size) {
> +			size = roundup_pow_of_two(size);
> +			if (size != orig_size) {
> +				sock_table =
> +				    vmalloc(RPS_SOCK_FLOW_TABLE_SIZE(size));
> +				if (!sock_table) {
> +					rcu_read_unlock();
> +					return -ENOMEM;
> +				}
> +
> +				sock_table->mask = size - 1;
> +			} else
> +				sock_table = orig_sock_table;
> +
> +			for (i = 0; i < size; i++)
> +				sock_table->ents[i] = RPS_NO_CPU;
> +		} else
> +			sock_table = NULL;
> +
> +		if (sock_table != orig_sock_table) {
> +			rcu_assign_pointer(rps_sock_flow_table, sock_table);
> +			synchronize_rcu();
> +			vfree(orig_sock_table);
> +		}
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return ret;
> +}
> +
>  /

It is not allowed to call vmalloc() inside rcu_read_unlock() section.

Anyway, rcu_read_unlock() is not appropriate (you want mutual exclusion
betwen concurrent writers here)

You should use a mutex here.
Or a spinlock (if you do the vmalloc()/vfree() things outside of the
locked section)