Messages in this thread | | | Subject | Re: [RFC PATCH] fs: use a sequence counter instead of file_lock in fd_install | From | Eric Dumazet <> | Date | Wed, 22 Apr 2015 06:55:15 -0700 |
| |
On Wed, 2015-04-22 at 15:31 +0200, Mateusz Guzik wrote: > On Tue, Apr 21, 2015 at 02:06:53PM -0700, Eric Dumazet wrote: > > On Tue, 2015-04-21 at 22:12 +0200, Mateusz Guzik wrote: > > > > > in dup_fd: > > > for (i = open_files; i != 0; i--) { > > > struct file *f = *old_fds++; > > > if (f) { > > > get_file(f); > > > > > > > I see no new requirement here. f is either NULL or not. > > multi threaded programs never had a guarantee dup_fd() would catch a non > > NULL pointer here. > > > > It's not about seeing NULL f or not, but using the right address for > dereference. > > If I read memory-barriers.txt right (see 'DATA DEPENDENCY BARRIERS'), it > is possible that cpus like alpha will see a non-NULL pointer and then > proceed to dereference *the old* (here: NULL) value. > > Hence I suspect this needs smp_read_barrier_depends (along with > ACCESS_ONCE). > > Other consumers (e.g. procfs code) use rcu_dereference macro which does > ends up using lockless_dereference macro, which in turn does: > #define lockless_dereference(p) \ > ({ \ > typeof(p) _________p1 = ACCESS_ONCE(p); \ > smp_read_barrier_depends(); /* Dependency order vs. p > above. */ \ > (_________p1); \ > }) > > That said memory barriers are not exactly my strong suit, but I do > believe my suspicion here is justified enough to ask someone with solid > memory barrier-fu to comment.
Again, your comment has nothing to do with the patch.
If there is old data, it only can be a NULL. And it is fine, case was _already_ handled.
It can not be an 'old' file pointer, because close() takes the spinlock. spin_unlock() contains a write memory barrier, so the NULL pointer put by close() would have been committed to memory.
This works also on alpha cpus.
| |