Re: [PATCH v9 2/3] mmc: core: add random fault injection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/9/14 Per Forlin <per.forlin@xxxxxxxxxx>:
> Hi Akinobu,
>
> On 13 September 2011 16:19, Per Forlin <per.forlin@xxxxxxxxxx> wrote:
>> On 13 September 2011 15:12, Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote:
>>> 2011/8/19 Per Forlin <per.forlin@xxxxxxxxxxxxxx>:
>>>
>>>> +#ifdef KERNEL
>>>> +/*
>>>> + * Internal function. Pass the boot param fail_mmc_request to
>>>> + * the setup fault injection attributes routine.
>>>> + */
>>>> +static int __init setup_fail_mmc_request(char *str)
>>>> +{
>>>> +       return setup_fault_attr(&fail_mmc_request, str);
>>>> +}
>>>> +__setup("fail_mmc_request=", setup_fail_mmc_request);
>>>> +#endif /* KERNEL */
>>>
>>> You attempt to enable __setup() only when mmc_core is built into
>>> the kernel.  Does it really work? I cannot find any drivers using
>>> "KERNEL" macro.
>>>
>> Your right it doesn't work. I think I change from ifndef MODULE to
>> ifdef KERNEL at one point.
>> It should be "ifndef MODULE"

It's simple and I like this solution.

module_param is more complicated than this. Also the parameter is only
usefull when when mmc_core is built into the kernel (it's useless when
mmc_core is built as a module).

>>> You can use module_param_cb() instead of __setup() without #ifdef KERNEL.
>>> When mmc_core is built into the kernel, you can specify the parameter
>>> with "mmc_core.fail_mmc_request=..."
>>>
> I am considering using module_param() with perm = 0 (not visible in
> sysfs). The purpose of the param is to set fault attributes during
> kerne boot time or module load time. After the kernel boot all can be
> set under debug fs, therefore no need to make the module param
> visible.
>
> What do you think about this? I have not tested it yet.
> ----------------------------------------------------------

...

> ----------------------------------------------------------
> It's only necessary to call setup_fault_attr() once for all hosts.
> Here it's called one time for each host. I think it's ok since the
> routine is small and used for debugging purposes.
> I could use a static bool to indicate whether setup_fault_attr() has
> already been issued.
> + if (fail_mmc_request && !setup_fault_attr_done)

module_param_cb() doesn't work as you expected?  (Although I suggested
to use #ifdef MODULE instead of #ifdef KERNEL, I'm just asking out of
curiosity)

static int fail_mmc_request_param_set(const char *val,
                                const struct kernel_param *kp)
{
        setup_fault_attr(&fail_mmc_request, val);
        return 0;
}

static const struct kernel_param_ops fail_mmc_request_param_ops = {
        .set = fail_mmc_request_param_set
};

module_param_cb(fail_mmc_request, &fail_mmc_request_param_ops,
                &fail_mmc_request, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux