* [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops
@ 2014-03-19 10:43 Richard Weinberger
2014-03-19 10:43 ` [PATCH 2/2] UBI: block: Implement kernel_param_ops->get() Richard Weinberger
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Richard Weinberger @ 2014-03-19 10:43 UTC (permalink / raw)
To: dedekind1
Cc: Richard Weinberger, linux-kernel, linux-mtd, ezequiel.garcia,
computersforpeace, dwmw2
You cannot mark these parameters as __initdata.
Otherwise the data is gone upon module exit.
Fixes:
[ 172.045465] BUG: unable to handle kernel paging request at ffffffffa001db38
[ 172.046020] IP: [<ffffffff81067aa4>] destroy_params+0x24/0x50
Signed-off-by: Richard Weinberger <richard@nod.at>
---
drivers/mtd/ubi/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 69a74fd..7ff473c 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -156,7 +156,7 @@ static int __init ubiblock_set_param(const char *val,
return 0;
}
-static struct kernel_param_ops ubiblock_param_ops __initdata = {
+static struct kernel_param_ops ubiblock_param_ops = {
.set = ubiblock_set_param,
};
module_param_cb(block, &ubiblock_param_ops, NULL, 0);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] UBI: block: Implement kernel_param_ops->get()
2014-03-19 10:43 [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Richard Weinberger
@ 2014-03-19 10:43 ` Richard Weinberger
2014-03-19 13:15 ` Ezequiel Garcia
2014-03-19 13:24 ` [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Ezequiel Garcia
2014-03-21 17:39 ` Artem Bityutskiy
2 siblings, 1 reply; 7+ messages in thread
From: Richard Weinberger @ 2014-03-19 10:43 UTC (permalink / raw)
To: dedekind1
Cc: Richard Weinberger, linux-kernel, linux-mtd, ezequiel.garcia,
computersforpeace, dwmw2
The ->get() function is not optional.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
drivers/mtd/ubi/block.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 7ff473c..9ef7df7 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -158,6 +158,7 @@ static int __init ubiblock_set_param(const char *val,
static struct kernel_param_ops ubiblock_param_ops = {
.set = ubiblock_set_param,
+ .get = param_get_charp,
};
module_param_cb(block, &ubiblock_param_ops, NULL, 0);
MODULE_PARM_DESC(block, "Attach block devices to UBI volumes. Parameter format: block=<path|dev,num|dev,name>.\n"
--
1.8.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] UBI: block: Implement kernel_param_ops->get()
2014-03-19 10:43 ` [PATCH 2/2] UBI: block: Implement kernel_param_ops->get() Richard Weinberger
@ 2014-03-19 13:15 ` Ezequiel Garcia
2014-03-19 13:23 ` Richard Weinberger
2014-03-19 13:26 ` Richard Weinberger
0 siblings, 2 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2014-03-19 13:15 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-mtd, computersforpeace, dwmw2, linux-kernel, dedekind1
Hi Richard,
On Mar 19, Richard Weinberger wrote:
> The ->get() function is not optional.
>
[..]
> static struct kernel_param_ops ubiblock_param_ops = {
> .set = ubiblock_set_param,
> + .get = param_get_charp,
> };
> module_param_cb(block, &ubiblock_param_ops, NULL, 0);
The comment for the function says they are both optional:
/**
* module_param_cb - general callback for a module/cmdline parameter
* @name: a valid C identifier which is the parameter name.
* @ops: the set & get operations for this parameter.
* @perm: visibility in sysfs.
*
* The ops can have NULL set or get functions.
*/
This has no visibility in sysfs, so I can't see how having a NULL
get() is a problem. What's the issue you're trying to fix here?
Maybe I'm missing something?
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] UBI: block: Implement kernel_param_ops->get()
2014-03-19 13:15 ` Ezequiel Garcia
@ 2014-03-19 13:23 ` Richard Weinberger
2014-03-19 13:26 ` Richard Weinberger
1 sibling, 0 replies; 7+ messages in thread
From: Richard Weinberger @ 2014-03-19 13:23 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: linux-mtd, computersforpeace, dwmw2, linux-kernel, dedekind1
Am 19.03.2014 14:15, schrieb Ezequiel Garcia:
> Hi Richard,
>
> On Mar 19, Richard Weinberger wrote:
>> The ->get() function is not optional.
>>
>
> [..]
>> static struct kernel_param_ops ubiblock_param_ops = {
>> .set = ubiblock_set_param,
>> + .get = param_get_charp,
>> };
>> module_param_cb(block, &ubiblock_param_ops, NULL, 0);
>
> The comment for the function says they are both optional:
>
> /**
> * module_param_cb - general callback for a module/cmdline parameter
> * @name: a valid C identifier which is the parameter name.
> * @ops: the set & get operations for this parameter.
> * @perm: visibility in sysfs.
> *
> * The ops can have NULL set or get functions.
> */
>
> This has no visibility in sysfs, so I can't see how having a NULL
> get() is a problem. What's the issue you're trying to fix here?
>
> Maybe I'm missing something?
>
Maybe I've misread the struct.
struct kernel_param_ops {
/* How the ops should behave */
unsigned int flags;
/* Returns 0, or -errno. arg is in kp->arg. */
int (*set)(const char *val, const struct kernel_param *kp);
/* Returns length written or -errno. Buffer is 4k (ie. be short!) */
int (*get)(char *buffer, const struct kernel_param *kp);
/* Optional function to free kp->arg when module unloaded. */
void (*free)(void *arg);
};
To me it looks like only ->free is optional.
Thanks,
//richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops
2014-03-19 10:43 [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Richard Weinberger
2014-03-19 10:43 ` [PATCH 2/2] UBI: block: Implement kernel_param_ops->get() Richard Weinberger
@ 2014-03-19 13:24 ` Ezequiel Garcia
2014-03-21 17:39 ` Artem Bityutskiy
2 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2014-03-19 13:24 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-mtd, computersforpeace, dwmw2, linux-kernel, dedekind1
On Mar 19, Richard Weinberger wrote:
> You cannot mark these parameters as __initdata.
> Otherwise the data is gone upon module exit.
>
> Fixes:
> [ 172.045465] BUG: unable to handle kernel paging request at ffffffffa001db38
> [ 172.046020] IP: [<ffffffff81067aa4>] destroy_params+0x24/0x50
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Thanks for the fix!
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] UBI: block: Implement kernel_param_ops->get()
2014-03-19 13:15 ` Ezequiel Garcia
2014-03-19 13:23 ` Richard Weinberger
@ 2014-03-19 13:26 ` Richard Weinberger
1 sibling, 0 replies; 7+ messages in thread
From: Richard Weinberger @ 2014-03-19 13:26 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: linux-mtd, computersforpeace, dwmw2, linux-kernel, dedekind1
Am 19.03.2014 14:15, schrieb Ezequiel Garcia:
> Hi Richard,
>
> On Mar 19, Richard Weinberger wrote:
>> The ->get() function is not optional.
>>
>
> [..]
>> static struct kernel_param_ops ubiblock_param_ops = {
>> .set = ubiblock_set_param,
>> + .get = param_get_charp,
>> };
>> module_param_cb(block, &ubiblock_param_ops, NULL, 0);
>
> The comment for the function says they are both optional:
>
> /**
> * module_param_cb - general callback for a module/cmdline parameter
> * @name: a valid C identifier which is the parameter name.
> * @ops: the set & get operations for this parameter.
> * @perm: visibility in sysfs.
> *
> * The ops can have NULL set or get functions.
> */
>
> This has no visibility in sysfs, so I can't see how having a NULL
> get() is a problem. What's the issue you're trying to fix here?
>
> Maybe I'm missing something?
>
BTW: I think we can omit ->get() as some other drivers also only have ->set().
Who want the credit for a documentation fix? ;)
Thanks,
//richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops
2014-03-19 10:43 [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Richard Weinberger
2014-03-19 10:43 ` [PATCH 2/2] UBI: block: Implement kernel_param_ops->get() Richard Weinberger
2014-03-19 13:24 ` [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Ezequiel Garcia
@ 2014-03-21 17:39 ` Artem Bityutskiy
2 siblings, 0 replies; 7+ messages in thread
From: Artem Bityutskiy @ 2014-03-21 17:39 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-mtd, computersforpeace, dwmw2, linux-kernel, ezequiel.garcia
On Wed, 2014-03-19 at 11:43 +0100, Richard Weinberger wrote:
> You cannot mark these parameters as __initdata.
> Otherwise the data is gone upon module exit.
>
> Fixes:
> [ 172.045465] BUG: unable to handle kernel paging request at ffffffffa001db38
> [ 172.046020] IP: [<ffffffff81067aa4>] destroy_params+0x24/0x50
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
Pushed to linux-ubifs.git, thanks!
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-21 17:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-19 10:43 [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Richard Weinberger
2014-03-19 10:43 ` [PATCH 2/2] UBI: block: Implement kernel_param_ops->get() Richard Weinberger
2014-03-19 13:15 ` Ezequiel Garcia
2014-03-19 13:23 ` Richard Weinberger
2014-03-19 13:26 ` Richard Weinberger
2014-03-19 13:24 ` [PATCH 1/2] UBI: block: Remove __initdata from ubiblock_param_ops Ezequiel Garcia
2014-03-21 17:39 ` Artem Bityutskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).