Re: md-cluster Module Requirement

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]

On Thu, Sep 01 2016, Guoqing Jiang wrote:

>
> Thanks, how about below changes?
>
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7619,20 +7619,19 @@ EXPORT_SYMBOL(unregister_md_cluster_operations);
>
>   int md_setup_cluster(struct mddev *mddev, int nodes)
>   {
> -       int err;
> -
> -       err = request_module("md-cluster");
> -       if (err) {
> -               pr_err("md-cluster module not found.\n");
> -               return -ENOENT;
> -       }
> -
>          spin_lock(&pers_lock);
> -       if (!md_cluster_ops || !try_module_get(md_cluster_mod)) {
> -               spin_unlock(&pers_lock);
> -               return -ENOENT;
> +       if (!md_cluster_ops) {
> +               /* load module and ensure it won't be unloaded */
> +               if (!request_module("md-cluster") &&

Calling request_module() under a spin_lock is not OK.  request_module()
needs to wait while the module is loaded.

Just call request_module, ignore the error, then take the spinlock and
see if the module is registered.

NeilBrown

> +                   !try_module_get(md_cluster_mod)) {
> +                       pr_err("md-cluster module found.\n");
> +                       spin_unlock(&pers_lock);
> +               } else {
> +                       pr_err("md-cluster module not found.\n");
> +                       spin_unlock(&pers_lock);
> +                       return -ENODEV;
> +               }
>          }
> -       spin_unlock(&pers_lock);
>
>          return md_cluster_ops->join(mddev, nodes);
>   }
>
>>>> I looked at linux-next and it appears this code is the same; is there
>>>> a test we can do before attempting to load the module in the case that
>>>> its built-in, or is there some other requirement that md-cluster needs
>>>> to be built as a module?
>>> Yes, we need some additional modules corosync/pacemaker and dlm,
>> That doesn't explain the error message though.
>
> You are right, I should read carefully :(
>
>> If MD_CLUSTER is built in, then DLM must be too.
>
> Sure,  then only need to ensure cluster is configured right.
>
> Best Regards,
> Guoqing

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]