[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200803112341.38005.nickpiggin@yahoo.com.au>
Date: Tue, 11 Mar 2008 23:41:37 +1100
From: Nick Piggin <nickpiggin@...oo.com.au>
To: Akinobu Mita <akinobu.mita@...il.com>
Cc: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: [PATCH 1/5] lib: introduce call_once()
On Tuesday 11 March 2008 01:57, Akinobu Mita wrote:
> +static inline int call_once(struct once_control *once_control,
> + int (*init_rouine)(void))
> +{
> + return likely(once_control->done) ? 0
> + : call_once_slow(once_control, init_rouine);
> +}
> +
> +#endif /* __LINUX_ONCE_H */
> Index: 2.6-rc/lib/once.c
> ===================================================================
> --- /dev/null
> +++ 2.6-rc/lib/once.c
> @@ -0,0 +1,18 @@
> +#include <linux/module.h>
> +#include <linux/once.h>
> +
> +int call_once_slow(struct once_control *once_control, int
> (*init_rouine)(void)) +{
> + int err = 0;
> +
> + mutex_lock(&once_control->lock);
> + if (!once_control->done) {
> + err = init_rouine();
> + if (!err)
> + once_control->done = 1;
> + }
> + mutex_unlock(&once_control->lock);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(call_once_slow);
The store "once_control->done = 1" can become visible before
init_routine has finished. The code after calling call_once may
also speculatively load some memory before the load of
once_control->done completes, so you can likewise have a data
race that way too.
To fix this, you need smp_wmb after init_rouine(), and probably
smp_mb() in the fastpath after the check but before returning.
Basically any time you have this situation where you're touching
a shared variable without using locks, then you're vastly
increasing the complexity of the code, and so you must have a
good reason for it.
So acquiring the mutex unconditionally would be the best way to
go, unless you're calling this a lot in fastpaths (in which case
I would say you should probably rework your code)
Thanks,
Nick
--
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/
Powered by blists - more mailing lists