Re: per cpun+ spin locks coexistence?

From: Jeremy Fitzhardinge
Date: Fri Mar 14 2008 - 13:56:34 EST


Peter Teoh wrote:
Help me out this one - in fs/file.c, there is a function free_fdtable_rcu():

void free_fdtable_rcu(struct rcu_head *rcu)
{
struct fdtable *fdt = container_of(rcu, struct fdtable, rcu);
struct fdtable_defer *fddef;

BUG_ON(!fdt);

if (fdt->max_fds <= NR_OPEN_DEFAULT) {
/*
* This fdtable is embedded in the files structure and that
* structure itself is getting destroyed.
*/
kmem_cache_free(files_cachep,
container_of(fdt, struct files_struct,
fdtab));
return;
}
if (fdt->max_fds <= (PAGE_SIZE / sizeof(struct file *))) {
kfree(fdt->fd);
kfree(fdt->open_fds);
kfree(fdt);
} else {
fddef = &get_cpu_var(fdtable_defer_list);
spin_lock(&fddef->lock);
fdt->next = fddef->next;
fddef->next = fdt;
/* vmallocs are handled from the workqueue context */
schedule_work(&fddef->wq);
spin_unlock(&fddef->lock);
put_cpu_var(fdtable_defer_list);
}
}

Notice above that get_cpu_var() is followed by spin_lock(). Does this
make sense? get_cpu_var() will return a variable that is only
accessible by the current CPU - guaranteed it will not be touch (read or
write) by another CPU, right?

No, not true. percpu is for stuff which is generally only touched by one CPU, but there's nothing stopping other processors from accessing it with per_cpu(var, cpu).

Besides, the lock isn't locking the percpu list head, but the thing on the head of the list, presumably to prevent races with the workqueue. (Though the list structure is nonstandard, so its not completely clear.)

J
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/