On Sat 16-12-17 16:14:07, Tetsuo Handa wrote:
> On 2017/12/16 8:04, Andrew Morton wrote:
> >> The implementation is steered toward an expensive slowpath, such as after
> >> the oom reaper has grabbed mm->mmap_sem of a still alive oom victim.
> >
> > some tweakage, please review.
> >
> > From: Andrew Morton <a...@linux-foundation.org>
> > Subject:
> > mm-mmu_notifier-annotate-mmu-notifiers-with-blockable-invalidate-callbacks-fix
> >
> > make mm_has_blockable_invalidate_notifiers() return bool, use
> > rwsem_is_locked()
> >
>
> > @@ -240,13 +240,13 @@ EXPORT_SYMBOL_GPL(__mmu_notifier_invalid
> > * Must be called while holding mm->mmap_sem for either read or write.
> > * The result is guaranteed to be valid until mm->mmap_sem is dropped.
> > */
> > -int mm_has_blockable_invalidate_notifiers(struct mm_struct *mm)
> > +bool mm_has_blockable_invalidate_notifiers(struct mm_struct *mm)
> > {
> > struct mmu_notifier *mn;
> > int id;
> > - int ret = 0;
> > + bool ret = false;
> >
> > - WARN_ON_ONCE(down_write_trylock(&mm->mmap_sem));
> > + WARN_ON_ONCE(!rwsem_is_locked(&mm->mmap_sem));
> >
> > if (!mm_has_notifiers(mm))
> > return ret;
>
> rwsem_is_locked() test isn't equivalent with __mutex_owner() == current test,
> is it?
> If rwsem_is_locked() returns true because somebody else has locked it, there
> is
> no guarantee that current thread has locked it before calling this function.
>
> down_write_trylock() test isn't equivalent with __mutex_owner() == current
> test, is it?
> What if somebody else held it for read or write (the worst case is
> registration path),
> down_write_trylock() will return false even if current thread has not locked
> it for
> read or write.
>
> I think this WARN_ON_ONCE() can not detect incorrect call to this function.
Yes it cannot catch _all_ cases. This is an inherent problem of
rwsem_is_locked because semaphores do not really have the owner concept.
The core idea behind this, I guess, is to catch obviously incorrect
usage and as such it gives us a reasonabe coverage. I could live without
the annotation but rwsem_is_locked looks better than down_write_trylock
to me.
--
Michal Hocko
SUSE Labs
