linux-kernel - Re: per cpun+ spin locks coexistence?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47DABBCE.5010803@goop.org>
Date:	Fri, 14 Mar 2008 10:54:22 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Peter Teoh <htmldeveloper@...il.com>
CC:	LKML <linux-kernel@...r.kernel.org>, Tejun Heo <htejun@...il.com>,
	Dipankar Sarma <dipankar@...ibm.com>
Subject: Re: per cpun+ spin locks coexistence?

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/