lkml.org 
[lkml]   [2008]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [patch] x86: some lock annotations for user copy paths
From
Date
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(&current->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(&current->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(&current->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.



\
 
 \ /
  Last update: 2008-09-10 17:03    [W:0.105 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site