Re: [RFC PATCH] slow-work: add (module*)work->owner to fix races with module clients

[David Howells]

Gregory Haskins <ghaskins@xxxxxxxxxx> wrote:

>   I found this while working on KVM.  I actually posted this patch with
> a KVM
> series yesterday and standalone earlier today, but neither seems to have
> made it to the lists.  I suspect there is an issue with git-mail/postfix
> on my system.

Also, your mail client has damaged the whitespace in the patch.

>  struct slow_work {
> +    struct module          *owner;

Can you add it to slow_work_ops instead?

>      work->ops->put_ref(work);
> +    barrier(); /* ensure that put_ref is not re-ordered with module_put =
> */
> +    module_put(work->owner);

Ummm...  Can it be?  module_put() and put_ref() are both out of line - surely
the compiler isn't allowed to reorder them?  If it's the CPU doing it then
barrier() isn't going to save you.

Note, however, that work may not be dereferenced like this after put_ref() is
called, unless you're sure that there's still a reference outstanding.

> +            if (!try_module_get(work->owner))
> +                goto cant_get_mod;

Note that this may result in a module getting stuck in unloading.  It may need
to do some work to complete the unload, and this will prevent that.

A better way might be to have put_ref() return, say, a pointer to a completion
struct, and if not NULL, have the caller of put_ref() call complete() on it.
That way you don't need to touch the module count, but can have something in
put_ref() keep track of when the last object is released and have its caller
invoke a completion to celebrate this fact.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/