Messages in this thread Patch in this message | | | Subject | Re: [patch] x86: some lock annotations for user copy paths | From | Peter Zijlstra <> | Date | Wed, 10 Sep 2008 17:01:04 +0200 |
| |
On Wed, 2008-09-10 at 16:48 +0200, Nick Piggin wrote:
> @@ -3016,3 +3016,18 @@ void print_vma_addr(char *prefix, unsign > } > up_read(¤t->mm->mmap_sem); > } > + > +void might_fault(void) > +{ > + /* > + * it would be nicer only to annotatea paths which are not under > + * pagefault_disable, however that requires a larger audit and > + * providing helpers like get_user_atomic. > + */ > + if (!in_atomic()) { > + might_sleep(); > + if (current->mm) > + might_lock_read(¤t->mm->mmap_sem); > + } > +} > +EXPORT_SYMBOL(might_fault);
>From the nitpick squad :-), I prefer the form:
void might_fault(void) { if (in_atomic()) return;
might_sleep();
if (!current->mm) might_lock_read(¤t->mm->mmap_sem); }
Due to it being one nesting level less.
> Index: linux-2.6/include/linux/kernel.h > =================================================================== > --- linux-2.6.orig/include/linux/kernel.h > +++ linux-2.6/include/linux/kernel.h > @@ -140,6 +140,15 @@ extern int _cond_resched(void); > (__x < 0) ? -__x : __x; \ > }) > > +#ifdef CONFIG_LOCKDEP > +void might_fault(void); > +#else > +static inline void might_fault(void) > +{ > + might_sleep(); > +} > +#endif > + > extern struct atomic_notifier_head panic_notifier_list; > extern long (*panic_blink)(long time); > NORET_TYPE void panic(const char * fmt, ...)
This forgets that in_atomic() again - possibly triggering might_sleep() where not appropriate.
I'm not sure its worth it to out-of-line the thing though (its only big on debug builds), and CONFIG_LOCKDEP is the wrong CONFIG_* variable, I think CONFIG_PROVE_LOCKING would be the appropriate one.
| |