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 04-17-2012, 07:25 AM
qiaonuohan
 
Default add -s option for struct command

Hello Dave,

The patch attached with this mail is to add "-s" option for struct
command. The output is like below.


crash> page.private,_mapcount.counter,lru.next -s ffffea0000000850
0 -1 0xffffea0000000878
crash> page.private,_mapcount.counter,lru.next -s ffffea0000000850 -h
0x0 0xffffffff 0xffffea0000000878

There are three features provided by the patches.

1. the data can be output in one line, easier to be parsed

2. it is extremly faster than original struct command and print command.
When using command like "struct xxx.yyy -s < address_list", if there are
lots of addresses in the file, address_list, the original struct command
will spend several minutes and even several hours. But the patch will
reduce the time to several seconds.


3. submember can also be output, like "_mapcount.counter" int the above
example


P.S.
The first patch is used to modified gdb. It add a command called
"printm", which can output the relative information of a struct's member.


--
--
Regards
Qiao Nuohan


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 04-18-2012, 01:21 PM
Dave Anderson
 
Default add -s option for struct command

----- Original Message -----
> Hello Dave,
>
> The patch attached with this mail is to add "-s" option for struct
> command. The output is like below.
>
> crash> page.private,_mapcount.counter,lru.next -s ffffea0000000850
> 0 -1 0xffffea0000000878
> crash> page.private,_mapcount.counter,lru.next -s ffffea0000000850 -h
> 0x0 0xffffffff 0xffffea0000000878
>
> There are three features provided by the patches.
>
> 1. the data can be output in one line, easier to be parsed
>
> 2. it is extremly faster than original struct command and print command.
> When using command like "struct xxx.yyy -s < address_list", if there are
> lots of addresses in the file, address_list, the original struct command
> will spend several minutes and even several hours. But the patch will
> reduce the time to several seconds.
>
> 3. submember can also be output, like "_mapcount.counter" int the above example
>
> P.S.
> The first patch is used to modified gdb. It add a command called
> "printm", which can output the relative information of a struct's
> member.

In our original discussions, i thought that I had made it clear that
the introduction of a new option paradigm with submembers could be
avoided by using, for example, "page._mapcounter" instead of having
to enter "page._mapcount.counter"? This option makes the struct
command seemingly violate its own rules, and really confuses things.
For example, with your patch, a user would see things like this:

crash> page._mapcount.counter ffffea0000000508 -s
-1
crash> page._mapcount.counter ffffea0000000508
struct: invalid format: page._mapcount.counter
crash> page._mapcount ffffea0000000508
_mapcount = {
counter = -1
}
crash> page._mapcount ffffea0000000508 -s
struct: invalid data structure reference page._mapcount
crash>

I had suggested that you look into the get_member_data() function
in to the gdb/symtab.c file to access the member offset and size
values.

I also don't like the idea of modifying the prototype of
the stalwart print_command_1() gdb function, and the creation
of a new gdb command. Whenever there is a need to update the
embedded gdb version, patches like this can be problematic.
I would prefer that you create a new "GNU_XXXX" #define,
similar to GNU_GET_SYMBOL_TYPE, pass the request through
the gdb_command_funnel switch statement, and write a new
standalone function to accomplish what you have done in the
print_command_1() function.

Dave



--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 04-19-2012, 09:19 AM
qiaonuohan
 
Default add -s option for struct command

At 2012-4-18 21:21, Dave Anderson wrote:

In our original discussions, i thought that I had made it clear that
the introduction of a new option paradigm with submembers could be
avoided by using, for example, "page._mapcounter" instead of having
to enter "page._mapcount.counter"? This option makes the struct
command seemingly violate its own rules, and really confuses things.
For example, with your patch, a user would see things like this:


The most important reason why I insisted this option is the performance.
Both original struct and print command are very slow. When kernel
debugger wants to parse a bit amount of data, the performance of
original struct and print command is not ideal.




crash> page._mapcount.counter ffffea0000000508 -s
-1
crash> page._mapcount.counter ffffea0000000508
struct: invalid format: page._mapcount.counter
crash> page._mapcount ffffea0000000508
_mapcount = {
counter = -1
}
crash> page._mapcount ffffea0000000508 -s
struct: invalid data structure reference page._mapcount
crash>


An idea of solving this confusion is changing the error information.
When users uses "-s" option and error happens, error information
suggests to use struct command without "-s" option if it is valid. And
vice versa, when error happens without specified "-s" option.




I had suggested that you look into the get_member_data() function
in to the gdb/symtab.c file to access the member offset and size
values.


Actually, the function need to be changed a lot to support what I want.
I need the information of submember, and I need the position and size of
bitfield. After investigation, function print_command_1 hides the data
that I want. I know it is not a good idea of modifying this function.
But what if a new function which has the similar mechanism with function
print_command_1?




I also don't like the idea of modifying the prototype of
the stalwart print_command_1() gdb function, and the creation
of a new gdb command. Whenever there is a need to update the
embedded gdb version, patches like this can be problematic.
I would prefer that you create a new "GNU_XXXX" #define,
similar to GNU_GET_SYMBOL_TYPE, pass the request through
the gdb_command_funnel switch statement, and write a new
standalone function to accomplish what you have done in the
print_command_1() function.



--
--
Regards
Qiao Nuohan



--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 04-19-2012, 12:50 PM
Dave Anderson
 
Default add -s option for struct command

----- Original Message -----
> At 2012-4-18 21:21, Dave Anderson wrote:
> > In our original discussions, i thought that I had made it clear
> > that
> > the introduction of a new option paradigm with submembers could be
> > avoided by using, for example, "page._mapcounter" instead of having
> > to enter "page._mapcount.counter"? This option makes the struct
> > command seemingly violate its own rules, and really confuses things.
> > For example, with your patch, a user would see things like this:
>
> The most important reason why I insisted this option is the performance.
> Both original struct and print command are very slow. When kernel
> debugger wants to parse a bit amount of data, the performance of
> original struct and print command is not ideal.
>
> >
> > crash> page._mapcount.counter ffffea0000000508 -s
> > -1
> > crash> page._mapcount.counter ffffea0000000508
> > struct: invalid format: page._mapcount.counter
> > crash> page._mapcount ffffea0000000508
> > _mapcount = {
> > counter = -1
> > }
> > crash> page._mapcount ffffea0000000508 -s
> > struct: invalid data structure reference page._mapcount
> > crash>
>
> An idea of solving this confusion is changing the error information.
> When users uses "-s" option and error happens, error information
> suggests to use struct command without "-s" option if it is valid.
> And vice versa, when error happens without specified "-s" option.

It's not so much the error message wording, it's the usage of a
completely different option-expression. And you can still display
the -s information without the extra submember.

> >
> > I had suggested that you look into the get_member_data() function
> > in to the gdb/symtab.c file to access the member offset and size
> > values.
>
> Actually, the function need to be changed a lot to support what I want.
> I need the information of submember, and I need the position and size of
> bitfield. After investigation, function print_command_1 hides the data
> that I want. I know it is not a good idea of modifying this function.
> But what if a new function which has the similar mechanism with function
> print_command_1?

Right, that's exactly what I suggested below:

> > I also don't like the idea of modifying the prototype of
> > the stalwart print_command_1() gdb function, and the creation
> > of a new gdb command. Whenever there is a need to update the
> > embedded gdb version, patches like this can be problematic.
> > I would prefer that you create a new "GNU_XXXX" #define,
> > similar to GNU_GET_SYMBOL_TYPE, pass the request through
> > the gdb_command_funnel switch statement, and write a new
> > standalone function to accomplish what you have done in the
> > print_command_1() function.

Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 05-22-2012, 09:32 AM
qiaonuohan
 
Default add -s option for struct command

Hello Dave,

After some many discussions, I rewrite the code to an extension module.
What I want is not only the member of structure page, but also other
structure's members. So for the speed of analysing a big number of data
and not obeying your attitude towards the change of struct command, I
made it an extension module.

And about the patch which changes gdb, I create a new method
print_command_2 to do the work.


At 2012-4-19 20:50, Dave Anderson wrote:



----- Original Message -----

At 2012-4-18 21:21, Dave Anderson wrote:

In our original discussions, i thought that I had made it clear
that
the introduction of a new option paradigm with submembers could be
avoided by using, for example, "page._mapcounter" instead of having
to enter "page._mapcount.counter"? This option makes the struct
command seemingly violate its own rules, and really confuses things.
For example, with your patch, a user would see things like this:


The most important reason why I insisted this option is the performance.
Both original struct and print command are very slow. When kernel
debugger wants to parse a bit amount of data, the performance of
original struct and print command is not ideal.



crash> page._mapcount.counter ffffea0000000508 -s
-1
crash> page._mapcount.counter ffffea0000000508
struct: invalid format: page._mapcount.counter
crash> page._mapcount ffffea0000000508
_mapcount = {
counter = -1
}
crash> page._mapcount ffffea0000000508 -s
struct: invalid data structure reference page._mapcount
crash>


An idea of solving this confusion is changing the error information.
When users uses "-s" option and error happens, error information
suggests to use struct command without "-s" option if it is valid.
And vice versa, when error happens without specified "-s" option.


It's not so much the error message wording, it's the usage of a
completely different option-expression. And you can still display
the -s information without the extra submember.



I had suggested that you look into the get_member_data() function
in to the gdb/symtab.c file to access the member offset and size
values.


Actually, the function need to be changed a lot to support what I want.
I need the information of submember, and I need the position and size of
bitfield. After investigation, function print_command_1 hides the data
that I want. I know it is not a good idea of modifying this function.
But what if a new function which has the similar mechanism with function
print_command_1?


Right, that's exactly what I suggested below:


I also don't like the idea of modifying the prototype of
the stalwart print_command_1() gdb function, and the creation
of a new gdb command. Whenever there is a need to update the
embedded gdb version, patches like this can be problematic.
I would prefer that you create a new "GNU_XXXX" #define,
similar to GNU_GET_SYMBOL_TYPE, pass the request through
the gdb_command_funnel switch statement, and write a new
standalone function to accomplish what you have done in the
print_command_1() function.


Dave

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





--
--
Regards
Qiao Nuohan


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 05-22-2012, 04:14 PM
Dave Anderson
 
Default add -s option for struct command

----- Original Message -----
> Hello Dave,
>
> After some many discussions, I rewrite the code to an extension module.
> What I want is not only the member of structure page, but also other
> structure's members. So for the speed of analysing a big number of data
> and not obeying your attitude towards the change of struct command, I
> made it an extension module.

Thank you -- I do appreciate that.

I will post "pstruct.c" on the extensions web page when crash-6.0.7 is released,
and will note the "crash-6.0.7 or later" restriction in the "comments" section
of its entry.

> And about the patch which changes gdb, I create a new method
> print_command_2 to do the work.

That addition looks fine to me, and does not affect any other commands.
Queued for crash-6.0.7.

Also, I should note that I have incorporated your "ipcs" command into
crash-6.0.7. It is essentially the same as your extension module, with
these exceptions:

(1) I have re-written the "help" page

(2) I replaced your default usage of the nsproxy of the CURRENT_TASK()
with that of pid 0, because the command fails if the current
task is exiting.

(3) Similarly to the "mount" command, I have added a "-n [pid|task]"
namespace option:

For kernels supporting namespaces, the -n option may be used to
display the IPC facilities with respect to the namespace of a
specified task:

-n pid a process PID.
-n task a hexadecimal task_struct pointer.

So congratulations are in order -- it is the first new command since 2001!

Thanks,
Dave


>
> At 2012-4-19 20:50, Dave Anderson wrote:
> >
> >
> > ----- Original Message -----
> >> At 2012-4-18 21:21, Dave Anderson wrote:
> >>> In our original discussions, i thought that I had made it clear
> >>> that
> >>> the introduction of a new option paradigm with submembers could
> >>> be
> >>> avoided by using, for example, "page._mapcounter" instead of
> >>> having
> >>> to enter "page._mapcount.counter"? This option makes the struct
> >>> command seemingly violate its own rules, and really confuses
> >>> things.
> >>> For example, with your patch, a user would see things like this:
> >>
> >> The most important reason why I insisted this option is the
> >> performance.
> >> Both original struct and print command are very slow. When kernel
> >> debugger wants to parse a bit amount of data, the performance of
> >> original struct and print command is not ideal.
> >>
> >>>
> >>> crash> page._mapcount.counter ffffea0000000508 -s
> >>> -1
> >>> crash> page._mapcount.counter ffffea0000000508
> >>> struct: invalid format: page._mapcount.counter
> >>> crash> page._mapcount ffffea0000000508
> >>> _mapcount = {
> >>> counter = -1
> >>> }
> >>> crash> page._mapcount ffffea0000000508 -s
> >>> struct: invalid data structure reference page._mapcount
> >>> crash>
> >>
> >> An idea of solving this confusion is changing the error
> >> information.
> >> When users uses "-s" option and error happens, error information
> >> suggests to use struct command without "-s" option if it is valid.
> >> And vice versa, when error happens without specified "-s" option.
> >
> > It's not so much the error message wording, it's the usage of a
> > completely different option-expression. And you can still display
> > the -s information without the extra submember.
> >
> >>>
> >>> I had suggested that you look into the get_member_data() function
> >>> in to the gdb/symtab.c file to access the member offset and size
> >>> values.
> >>
> >> Actually, the function need to be changed a lot to support what I
> >> want.
> >> I need the information of submember, and I need the position and
> >> size of
> >> bitfield. After investigation, function print_command_1 hides the
> >> data
> >> that I want. I know it is not a good idea of modifying this
> >> function.
> >> But what if a new function which has the similar mechanism with
> >> function
> >> print_command_1?
> >
> > Right, that's exactly what I suggested below:
> >
> >>> I also don't like the idea of modifying the prototype of
> >>> the stalwart print_command_1() gdb function, and the creation
> >>> of a new gdb command. Whenever there is a need to update the
> >>> embedded gdb version, patches like this can be problematic.
> >>> I would prefer that you create a new "GNU_XXXX" #define,
> >>> similar to GNU_GET_SYMBOL_TYPE, pass the request through
> >>> the gdb_command_funnel switch statement, and write a new
> >>> standalone function to accomplish what you have done in the
> >>> print_command_1() function.
> >
> > Dave
> >
> > --
> > Crash-utility mailing list
> > Crash-utility@redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> >
> >
>
>
> --
> --
> Regards
> Qiao Nuohan
>
>
>
> --
> 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 10:49 AM.

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