On Mon, Dec 13, 2010 at 6:25 PM, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote: > Le lundi 13 décembre 2010 à 13:42 +1100, Nick Piggin a écrit : >> + */ >> +static inline void add_mnt_count(struct vfsmount *mnt, int n) > > maybe name it __add_mnt_count() (should be called with preempt off) > >> +{ >> +#ifdef CONFIG_SMP >> + (*per_cpu_ptr(mnt->mnt_count, smp_processor_id())) += n; > > __this_cpu_add(mnt->mnt_count, n); > >> +#else >> + mnt->mnt_count += n; >> +#endif >> +} > > and define a preempt safe version : > > static inline void __add_mnt_count(struct vfsmount *mnt, int n) > { > #ifdef CONFIG_SMP > __this_cpu_add(mnt->mnt_count, n); > #else > mnt->mnt_count += n; > #endif > } > > static inline void add_mnt_count(struct vfsmount *mnt, int n) > { > #ifdef CONFIG_SMP > this_cpu_add(mnt->mnt_count, n); > #else > preempt_disable(); > mnt->mnt_count += n; > preempt_enable(); > #endif > } That looks good, thanks. >> + >> +static inline void set_mnt_count(struct vfsmount *mnt, int n) >> +{ >> +#ifdef CONFIG_SMP > > >> + preempt_disable(); >> + (*per_cpu_ptr(mnt->mnt_count, smp_processor_id())) = n; >> + preempt_enable(); > > last 3 lines can be replaced by : > > this_cpu_write(mnt->mnt_count, n); Yep, thanks. >> +#else >> + mnt->mnt_count = n; >> +#endif >> +} >> + > >> #ifdef CONFIG_SMP >> int __percpu *mnt_writers; >> + int __percpu *mnt_count; >> #else >> int mnt_writers; >> + int mnt_count; >> #endif > > You could use a struct and one per cpu allocation to use one cache line > for both objects : > > struct mnt_counters { > int writers; > int count; > }; > > ... > > #ifdef CONFIG_SMP > struct mnt_counters __percpu *mnt_counters; > #else > struct mnt_counters mnt_counters; > #endif > > This would use one pointer instead of two in SMP Yes that's a good point too. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html