* Sasha Levin <sasha.le...@oracle.com> wrote:

> We can rather easily make lockdep work from userspace, although 3 issues
> remain which I'm not sure about:
> 
>  - Kernel naming - we can just wrap init_utsname() to return kvmtool related
> utsname, is that what we want though?
> 
>  - static_obj() - I don't have a better idea than calling mprobe(), which 
> sounds
> wrong as well.
> 
>  - debug_show_all_locks() - we don't actually call it from userspace yet, but 
> I think
> we might want to, so I'm not sure how to make it pretty using existing kernel 
> code.
> 
> Signed-off-by: Sasha Levin <sasha.le...@oracle.com>
> ---
>  kernel/lockdep.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/lockdep.c b/kernel/lockdep.c
> index 7981e5b..fdd3670 100644
> --- a/kernel/lockdep.c
> +++ b/kernel/lockdep.c
> @@ -567,10 +567,12 @@ static void lockdep_print_held_locks(struct task_struct 
> *curr)
>  
>  static void print_kernel_ident(void)
>  {
> +#ifdef __KERNEL__
>       printk("%s %.*s %s\n", init_utsname()->release,
>               (int)strcspn(init_utsname()->version, " "),
>               init_utsname()->version,
>               print_tainted());
> +#endif

I guess wrapping init_utsname() is not worth it. Although 
kvmtool could provide the host system's utsname - kernel 
identity is useful for debugging info.

You could generate a Git hash version string like tools/perf/ 
does (see PERF_VERSION and tools/perf/util/PERF-VERSION-GEN), 
and put that into the ->version field.

->release could be the kvmtool version, and print_tainted() 
could return an empty string.

That way you could provide init_utsname() and could remove this 
#ifdef.

>  }
>  
>  static int very_verbose(struct lock_class *class)
> @@ -586,6 +588,7 @@ static int very_verbose(struct lock_class *class)
>   */
>  static int static_obj(void *obj)
>  {
> +#ifdef __KERNEL__
>       unsigned long start = (unsigned long) &_stext,
>                     end   = (unsigned long) &_end,
>                     addr  = (unsigned long) obj;
> @@ -609,6 +612,8 @@ static int static_obj(void *obj)
>        * module static or percpu var?
>        */
>       return is_module_address(addr) || is_module_percpu_address(addr);
> +#endif
> +     return 1;

Could you put an:

#ifndef static_obj

around it? Then kvmtool could define its own trivial version of 
static_obj():

  #define static_obj(x) 1U

or so.

> @@ -4108,7 +4113,7 @@ void debug_check_no_locks_held(struct task_struct *task)
>       if (unlikely(task->lockdep_depth > 0))
>               print_held_locks_bug(task);
>  }
> -
> +#ifdef __KERNEL__
>  void debug_show_all_locks(void)
>  {
>       struct task_struct *g, *p;

I guess a show-all-locks functionality would be useful to 
kvmtool as well?

Thanks,

        Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to