On Sun, Dec 14, 2014 at 03:14:29AM +0000, Al Viro wrote: > On Sat, Dec 13, 2014 at 05:35:17PM -0800, Linus Torvalds wrote: > > On Sat, Dec 13, 2014 at 4:33 PM, Al Viro <v...@zeniv.linux.org.uk> wrote: > > > > > > So does SMP - this_cpu_dec() relies on preemption being disabled. > > > > No. really. It very much does not. Not on x86, not elsewhere. It's > > part of the whole point of "this_cpu_p()". They are preemption and > > interrupt safe. > > > > It's the "__this_cpu_op()" ones that need external protection. > > Right you are - I really need to get some coffee... Sorry... > > FWIW, do we need to disable interrupts there? After all, mnt_want_write() > and mnt_drop_write() shouldn't be done from interrupt context - they can > happen via schedule_delayed_work(), but that's it...
OK, having looked through the tree - we really don't need to bother with disabling interrupts (fortunately - or UP case would be broken). So how about turning those into __this_cpu_{inc,dec} and yes, moving preempt disabling into mnt_{inc,dec}_writers()? Like this: diff --git a/fs/namespace.c b/fs/namespace.c index 5b66b2b..48cb162 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -274,20 +274,32 @@ EXPORT_SYMBOL_GPL(__mnt_is_readonly); static inline void mnt_inc_writers(struct mount *mnt) { + preempt_disable(); #ifdef CONFIG_SMP - this_cpu_inc(mnt->mnt_pcp->mnt_writers); + __this_cpu_inc(mnt->mnt_pcp->mnt_writers); #else mnt->mnt_writers++; #endif + preempt_enable(); } -static inline void mnt_dec_writers(struct mount *mnt) +/** + * __mnt_drop_write - give up write access to a mount + * @mnt: the mount on which to give up write access + * + * Tells the low-level filesystem that we are done + * performing writes to it. Must be matched with + * __mnt_want_write() call above. + */ +void __mnt_drop_write(struct vfsmount *m) { + preempt_disable(); #ifdef CONFIG_SMP - this_cpu_dec(mnt->mnt_pcp->mnt_writers); + __this_cpu_dec(real_mount(m)->mnt_pcp->mnt_writers); #else - mnt->mnt_writers--; + real_mount(m)->mnt_writers--; #endif + preempt_enable(); } static unsigned int mnt_get_writers(struct mount *mnt) @@ -336,7 +348,6 @@ int __mnt_want_write(struct vfsmount *m) struct mount *mnt = real_mount(m); int ret = 0; - preempt_disable(); mnt_inc_writers(mnt); /* * The store to mnt_inc_writers must be visible before we pass @@ -353,10 +364,9 @@ int __mnt_want_write(struct vfsmount *m) */ smp_rmb(); if (mnt_is_readonly(m)) { - mnt_dec_writers(mnt); + __mnt_drop_write(m); ret = -EROFS; } - preempt_enable(); return ret; } @@ -399,9 +409,7 @@ int mnt_clone_write(struct vfsmount *mnt) /* superblock may be r/o */ if (__mnt_is_readonly(mnt)) return -EROFS; - preempt_disable(); mnt_inc_writers(real_mount(mnt)); - preempt_enable(); return 0; } EXPORT_SYMBOL_GPL(mnt_clone_write); @@ -441,21 +449,6 @@ int mnt_want_write_file(struct file *file) EXPORT_SYMBOL_GPL(mnt_want_write_file); /** - * __mnt_drop_write - give up write access to a mount - * @mnt: the mount on which to give up write access - * - * Tells the low-level filesystem that we are done - * performing writes to it. Must be matched with - * __mnt_want_write() call above. - */ -void __mnt_drop_write(struct vfsmount *mnt) -{ - preempt_disable(); - mnt_dec_writers(real_mount(mnt)); - preempt_enable(); -} - -/** * mnt_drop_write - give up write access to a mount * @mnt: the mount on which to give up write access * -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/