FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Crash Utility

 
 
LinkBack Thread Tools
 
Old 01-17-2011, 09:04 AM
Toshikazu Nakayama
 
Default The new API about ikconfig.

Hi,

I would like to suggest new ikconfig's API from defs.h.

The get_kernel_config("config name without CONFIG_ prefix", &val)
will return one of target kernel configurations.
If val == NULL, return value is poinited whether config is valid or not.
IKCONFIG_N: kernel config is not set (not valid).
IKCONFIG_Y: kernel config is set y (valid).
IKCONFIG_M: kernel config is set m (valid).
IKCONFIG_STR: kernel config is values or strings, etc (valid).

API require CONFIG_IKCONFIG=y. Otherwise user is warned.

I think it is useful for such as extension module developers
to write kernel config switch code.

Although ifdef or symbol APIs can be used instead,
I believe using this API can write more impressive or flexible code
about kernel config depended parts (over my senses...).

Thanks,
Toshi.
-----------------------------------------------------------

Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.

[ code of kernel-2.6.35 ]

#define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)

[ code of crash utility or extension modules ]

static int radix_tree_map_shift = 6;
#define RADIX_TREE_MAP_SHIFT radix_tree_map_shift

int _init(void)
{
read_in_kernel_config(IKCFG_SETUP);

if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
radix_tree_map_shift = 4;

return 0;
}

int int _fini(void)
{
read_in_kernel_config(IKCFG_FREE);

return 0;
}

Toshikazu Nakayama (2):
new ikconfig API.
dump_kernel_table()

crash-5.1.1/defs.h | 13 ++++
crash-5.1.1/kernel.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 178 insertions(+), 1 deletions(-)

--
1.7.4.rc2
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-17-2011, 03:59 PM
Dave Anderson
 
Default The new API about ikconfig.

----- Original Message -----
> Hi,
>
> I would like to suggest new ikconfig's API from defs.h.
>
> The get_kernel_config("config name without CONFIG_ prefix", &val)
> will return one of target kernel configurations.
> If val == NULL, return value is poinited whether config is valid or
> not.
> IKCONFIG_N: kernel config is not set (not valid).
> IKCONFIG_Y: kernel config is set y (valid).
> IKCONFIG_M: kernel config is set m (valid).
> IKCONFIG_STR: kernel config is values or strings, etc (valid).
>
> API require CONFIG_IKCONFIG=y. Otherwise user is warned.
>
> I think it is useful for such as extension module developers
> to write kernel config switch code.
>
> Although ifdef or symbol APIs can be used instead,
> I believe using this API can write more impressive or flexible code
> about kernel config depended parts (over my senses...).

This does look like a good idea for extension modules. I'm off today,
but will check your patches out tomorrow.

Thanks,
Dave


> Thanks,
> Toshi.
> -----------------------------------------------------------
>
> Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
>
> [ code of kernel-2.6.35 ]
>
> #define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
>
> [ code of crash utility or extension modules ]
>
> static int radix_tree_map_shift = 6;
> #define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
>
> int _init(void)
> {
> read_in_kernel_config(IKCFG_SETUP);
>
> if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
> radix_tree_map_shift = 4;
>
> return 0;
> }
>
> int int _fini(void)
> {
> read_in_kernel_config(IKCFG_FREE);
>
> return 0;
> }
>
> Toshikazu Nakayama (2):
> new ikconfig API.
> dump_kernel_table()
>
> crash-5.1.1/defs.h | 13 ++++
> crash-5.1.1/kernel.c | 166
> +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 178 insertions(+), 1 deletions(-)
>
> --
> 1.7.4.rc2
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-19-2011, 04:12 PM
Dave Anderson
 
Default The new API about ikconfig.

----- Original Message -----
> Hi,
>
> I would like to suggest new ikconfig's API from defs.h.
>
> The get_kernel_config("config name without CONFIG_ prefix", &val)
> will return one of target kernel configurations.
> If val == NULL, return value is poinited whether config is valid or
> not.
> IKCONFIG_N: kernel config is not set (not valid).
> IKCONFIG_Y: kernel config is set y (valid).
> IKCONFIG_M: kernel config is set m (valid).
> IKCONFIG_STR: kernel config is values or strings, etc (valid).
>
> API require CONFIG_IKCONFIG=y. Otherwise user is warned.
>
> I think it is useful for such as extension module developers
> to write kernel config switch code.
>
> Although ifdef or symbol APIs can be used instead,
> I believe using this API can write more impressive or flexible code
> about kernel config depended parts (over my senses...).
>
> Thanks,
> Toshi.

Hello Toshi,

I have a few issues/questions with your patch:

1. This should be impossible, because even if preload_extensions()
is used, that function is called long after the init-time call
to read_in_kernel_config(IKCFG_INIT) is done:

+ if (command == IKCFG_INIT && kt->ikconfig_setup) {
+ /* fastpath */
+ if (get_kernel_config("DEBUG_BUGVERBOSE", NULL) == IKCONFIG_N)
+ kt->flags |= BUGVERBOSE_OFF;
+
+ if (get_kernel_config("NR_CPUS", &val) == IKCONFIG_STR) {
+ kt->kernel_NR_CPUS = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_NR_CPUS: %d
",
+ kt->kernel_NR_CPUS);
+ }
+ if (get_kernel_config("PGTABLE_4", NULL) == IKCONFIG_Y) {
+ machdep->flags |= VM_4_LEVEL;
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_PGTABLE_4
");
+ }
+ if (get_kernel_config("HZ", &val) == IKCONFIG_STR) {
+ machdep->hz = atoi(val);
+ if (CRASHDEBUG(1))
+ error(INFO, "CONFIG_HZ: %d
", machdep->hz);
+ }
+
+ goto out1;
+ }

2. I didn't test this, but I think this will be a problem because it will
not return back to the _init() function in the extension module:

if ((sp = symbol_search("kernel_config_data")) == NULL) {
- if (command == IKCFG_READ)
+ if (command == IKCFG_READ || command == IKCFG_SETUP)
error(FATAL,
"kernel_config_data does not exist in this kernel
");
return;

It should do this:

If IKCONFIG_INIT: return quietly with no error message
If IKCONFIG_READ: error(FATAL, ...
If IKCONFIG_SETUP: error(WARNING, ...

2. Can you explain the need for the IKCONFIG_SORT_INTERVAL stuff?

3. When adding items to any global structure (such as to the kernel_table),
I always add them to the end of the structure so that any pre-existing
extension modules that use something in the unmodified structure will
still work even if additional entries are added to the end.

4. Just a preference on my part, but I would allow get_kernel_config()
to accept both "CONFIG_XXXX" and "XXX" as a "name" argument, and then
just strip off "CONFIG_" if it's there.

Dave


> -----------------------------------------------------------
>
> Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
>
> [ code of kernel-2.6.35 ]
>
> #define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
>
> [ code of crash utility or extension modules ]
>
> static int radix_tree_map_shift = 6;
> #define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
>
> int _init(void)
> {
> read_in_kernel_config(IKCFG_SETUP);
>
> if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
> radix_tree_map_shift = 4;
>
> return 0;
> }
>
> int int _fini(void)
> {
> read_in_kernel_config(IKCFG_FREE);
>
> return 0;
> }
>
> Toshikazu Nakayama (2):
> new ikconfig API.
> dump_kernel_table()
>
> crash-5.1.1/defs.h | 13 ++++
> crash-5.1.1/kernel.c | 166
> +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 178 insertions(+), 1 deletions(-)
>
> --
> 1.7.4.rc2
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-20-2011, 03:57 AM
Toshikazu Nakayama
 
Default The new API about ikconfig.

Hi Dave,

I shuold agree with your comments at all, thanks a lot.

>Hello Toshi,
>
>I have a few issues/questions with your patch:
>
>1. This should be impossible, because even if preload_extensions()
> is used, that function is called long after the init-time call
> to read_in_kernel_config(IKCFG_INIT) is done:

You're right. I'll remove this condition because this never hit as TRUE.

>+ if (command == IKCFG_INIT && kt->ikconfig_setup) {
>+ /* fastpath */
>+ if (get_kernel_config("DEBUG_BUGVERBOSE", NULL) == IKCONFIG_N)
>+ kt->flags |= BUGVERBOSE_OFF;
>+
>+ if (get_kernel_config("NR_CPUS", &val) == IKCONFIG_STR) {
>+ kt->kernel_NR_CPUS = atoi(val);
>+ if (CRASHDEBUG(1))
>+ error(INFO, "CONFIG_NR_CPUS: %d
",
>+ kt->kernel_NR_CPUS);
>+ }
>+ if (get_kernel_config("PGTABLE_4", NULL) == IKCONFIG_Y) {
>+ machdep->flags |= VM_4_LEVEL;
>+ if (CRASHDEBUG(1))
>+ error(INFO, "CONFIG_PGTABLE_4
");
>+ }
>+ if (get_kernel_config("HZ", &val) == IKCONFIG_STR) {
>+ machdep->hz = atoi(val);
>+ if (CRASHDEBUG(1))
>+ error(INFO, "CONFIG_HZ: %d
", machdep->hz);
>+ }
>+
>+ goto out1;
>+ }
>
>2. I didn't test this, but I think this will be a problem because it will
> not return back to the _init() function in the extension module:
>
> if ((sp = symbol_search("kernel_config_data")) == NULL) {
>- if (command == IKCFG_READ)
>+ if (command == IKCFG_READ || command == IKCFG_SETUP)
> error(FATAL,
> "kernel_config_data does not exist in this kernel
");
> return;
>
> It should do this:
>
> If IKCONFIG_INIT: return quietly with no error message
> If IKCONFIG_READ: error(FATAL, ...
> If IKCONFIG_SETUP: error(WARNING, ...

I'll change with this manner.

>2. Can you explain the need for the IKCONFIG_SORT_INTERVAL stuff?

Example for CONFIG_X86(_64) which is probably referenced many times from
extensions, it shuold be in front position of config list for rapid loop break.
In my environs, kernel owned around 400 valid kernel configs.

However, I'll remove them except kt->ikconfig_refs.

Extensions may not repeat get_kernel_config(), it's only in their initialization.
Also such a major configs are already existing at front of kernel_config_data.
(In addition to these reasons, can not work for "is not set" configs.)

The kt->ikconfig_refs is remained as get_kernel_config() call (reference)
counters including the case of IKCONFIG_N.

>3. When adding items to any global structure (such as to the kernel_table),
> I always add them to the end of the structure so that any pre-existing
> extension modules that use something in the unmodified structure will
> still work even if additional entries are added to the end.

I could not be aware of this issue, will fix my bad-mannered.
In my understanding, this is not indicating position
at dump_kernel_table(), correct?

>4. Just a preference on my part, but I would allow get_kernel_config()
> to accept both "CONFIG_XXXX" and "XXX" as a "name" argument, and then
> just strip off "CONFIG_" if it's there.

I also prefer this idea, I'll update patch to support "CONFIG_name" argument.

Thanks,
Toshi.

>Dave
>
>
>> -----------------------------------------------------------
>>
>> Example usage: Resolve RADIX_TREE_MAP_SHIFT from config depends.
>>
>> [ code of kernel-2.6.35 ]
>>
>> #define RADIX_TREE_MAP_SHIFT (CONFIG_BASE_SMALL ? 4 : 6)
>>
>> [ code of crash utility or extension modules ]
>>
>> static int radix_tree_map_shift = 6;
>> #define RADIX_TREE_MAP_SHIFT radix_tree_map_shift
>>
>> int _init(void)
>> {
>> read_in_kernel_config(IKCFG_SETUP);
>>
>> if (get_kernel_config("BASE_SMALL", NULL) == IKCONFIG_Y)
>> radix_tree_map_shift = 4;
>>
>> return 0;
>> }
>>
>> int int _fini(void)
>> {
>> read_in_kernel_config(IKCFG_FREE);
>>
>> return 0;
>> }
>>
>> Toshikazu Nakayama (2):
>> new ikconfig API.
>> dump_kernel_table()
>>
>> crash-5.1.1/defs.h | 13 ++++
>> crash-5.1.1/kernel.c | 166
>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 178 insertions(+), 1 deletions(-)
>>
>> --
>> 1.7.4.rc2
>>
>> --
>> Crash-utility mailing list
>> Crash-utility@redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>
>--
>Crash-utility mailing list
>Crash-utility@redhat.com
>https://www.redhat.com/mailman/listinfo/crash-utility

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 

Thread Tools




All times are GMT. The time now is 08:25 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org