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-20-2012, 03:38 AM
Wen Congyang
 
Default introduce a new command to display the disk's information

Hi, Dave

When we investigate the problems of disk I/O, we want to get the disk's
gendisk address and request queue's address easily, and the requests num
is also important.

Tha attached patch introduce a new command diskio to display such information.

Thanks
Wen Congyang
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-20-2012, 07:24 PM
Dave Anderson
 
Default introduce a new command to display the disk's information

----- Original Message -----
> Hi, Dave
>
> When we investigate the problems of disk I/O, we want to get the disk's
> gendisk address and request queue's address easily, and the requests num
> is also important.
>
> Tha attached patch introduce a new command diskio to display such
> information.
>
> Thanks
> Wen Congyang

Hello Wen,

I built and tested this patch, and it certainly contains some useful
information. However, I do have a several suggestions.

First, there's no need to make it a unique command. It would be
far more appropriate to make it a "dev" command option, which for
example, already shows gendisk structure addresses for each of the
block devices.

So please move all of your functions from kernel.c to dev.c.
Then, for example, use "dev -d", and have cmd_dev() call your
command like this:

while ((c = getopt(argcnt, args, "dpi")) != EOF) {
switch(c)
{
case 'd':
diskio_option();
return;
...

Since all of the new offset_table and size_table entries are only
used by this command, you can avoid unnecessarily initializing
everything in kernel_init() by doing something like this:

static void
diskio_option(void)
{
if (INVALID_MEMBER(class_devices)) {
MEMBER_OFFSET_INIT(class_devices, "class", "class_devices");
if (INVALID_MEMBER(class_devices))
MEMBER_OFFSET_INIT(class_devices, "class", "devices");
MEMBER_OFFSET_INIT(class_p, "class", "p");
MEMBER_OFFSET_INIT(class_private_devices, "class_private",
"class_devices");
MEMBER_OFFSET_INIT(device_knode_class, "device", "knode_class");
MEMBER_OFFSET_INIT(device_node, "device", "node");
MEMBER_OFFSET_INIT(device_type, "device", "type");
MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "dev");
if (INVALID_MEMBER(gendisk_dev))
MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "__dev");
MEMBER_OFFSET_INIT(gendisk_kobj, "gendisk", "kobj");
MEMBER_OFFSET_INIT(gendisk_part0, "gendisk", "part0");
MEMBER_OFFSET_INIT(gendisk_queue, "gendisk", "queue");
MEMBER_OFFSET_INIT(hd_struct_dev, "hd_struct", "__dev");
MEMBER_OFFSET_INIT(klist_k_list, "klist", "k_list");
MEMBER_OFFSET_INIT(klist_node_n_klist, "klist_node", "n_klist");
MEMBER_OFFSET_INIT(klist_node_n_node, "klist_node", "n_node");
MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
MEMBER_OFFSET_INIT(kset_list, "kset", "list");
MEMBER_OFFSET_INIT(request_list_count, "request_list", "count");
MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
"in_flight");
MEMBER_OFFSET_INIT(request_queue_rq, "request_queue", "rq");
MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
"klist_devices");
MEMBER_OFFSET_INIT(subsystem_kset, "subsystem", "kset");
STRUCT_SIZE_INIT(subsystem, "subsystem");
STRUCT_SIZE_INIT(class_private, "class_private");
MEMBER_SIZE_INIT(rq_in_flight, "request_queue", "in_flight");
MEMBER_SIZE_INIT(class_private_devices, "class_private",
"class_devices");
}

display_all_diskio();
}

Secondly, the whole READ/WRITE issue is confusing to the uninitiated
(like myself). After speaking with Jeff Moyer, we believe the output
of your command could be clarified.

Your help page indicates:

+" This command dumps I/O statistics of all disks:",
+" TOTAL: The total requests that have not been ended",
+" READ: The total read requests that have not been ended",
+" WRITE: The total write requests that have not been ended",
+" DRV: The total requests that have been in the driver, but not end",
+"",
+" Note: some kernel does not contain read/write requests, and the command",
+" will output '-----'"
+"
EXAMPLES",
+" %s> diskio",
+" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
+" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
+" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
+" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
+" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
+" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
+NULL

As Jeff explained to me, this is how it works:

(1) In older kernels, this enum did not exist:

enum {
BLK_RW_ASYNC = 0,
BLK_RW_SYNC = 1,
};

and in that case, the request_list.count[2] values are the
READ/WRITE values, as you show in the help page example above.

(2) In newer kernels, the enum does exist, and the meaning of the
request_list.count[2] values are ASYNC/SYNC requests. In that
case, you show "-----" under the READ and WRITE columns, which
I found *very* confusing.

What Jeff Moyer suggested is that -- in the case of new kernels -- you
should alternatively show the counts with ASYNC and SYNC columns like
this:

MAJOR GENDISK NAME REQUEST QUEUE TOTAL ASYNC SYNC DRV",
...

And the READ/WRITE vs. ASYNC/SYNC output display difference should be
referenced in the help page output.

Third, a minor nit -- note that you misspelled it as "RUQUEST" both in
the command and in the help page. Also, it appears that you used an IA64 as
the example, given the GENDISK addresses. But aren't the REQUEST QUEUE
addresses below beginning with "0x300..." user-space addresses for IA64?

+" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
+" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
+" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
+" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
+" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
+" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",

In any case, can you change it to use either x86_64 or x86 as an example?

Fourth, in testing this with a 2.6.25 kernel, the command hangs:

crash> dev -d
MAJOR GENDISK NAME REQUEST QUEUE TOTAL READ WRITE DRV
2 0xffff81012d8a5000 fd0 0xffff81012dc053c0 0 0 0 0
22 0xffff81012dc6b000 hdc 0xffff81012d8ae340 0 0 0 0
8 0xffff81012dd71000 sda 0xffff81012d8af040 0 0 0 0
< hangs here forever >

I'm not absolutely sure, but if I hit CTRL-C under gdb, it seems that it's
spinning in next_disk_list() in that "goto again" loop?

Fifth, I tried it on a much older RHEL3 kernel, which shows:

crash> dev -d
dev: invalid request_queue.in_flight's size
crash>

It's not really invalid size, but it's more the case that you
don't support that old a kernel version. In that case, instead
of doing this:

error(FATAL, "invalid request_queue.in_flight's size
");

you should do this instead:

option_not_supported('d');

And finally, whenever adding fields to the offset_table or size_table,
please display their values in dump_offset_table() for the "help -o"
command.

Thanks,
Dave




--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-30-2012, 01:08 AM
Wen Congyang
 
Default introduce a new command to display the disk's information

Hi, Dave

At 01/21/2012 04:24 AM, Dave Anderson Wrote:
>
>
> ----- Original Message -----
>> Hi, Dave
>>
>> When we investigate the problems of disk I/O, we want to get the disk's
>> gendisk address and request queue's address easily, and the requests num
>> is also important.
>>
>> Tha attached patch introduce a new command diskio to display such
>> information.
>>
>> Thanks
>> Wen Congyang
>
> Hello Wen,
>
> I built and tested this patch, and it certainly contains some useful
> information. However, I do have a several suggestions.
>
> First, there's no need to make it a unique command. It would be
> far more appropriate to make it a "dev" command option, which for
> example, already shows gendisk structure addresses for each of the
> block devices.
>
> So please move all of your functions from kernel.c to dev.c.
> Then, for example, use "dev -d", and have cmd_dev() call your
> command like this:
>
> while ((c = getopt(argcnt, args, "dpi")) != EOF) {
> switch(c)
> {
> case 'd':
> diskio_option();
> return;
> ...
>
> Since all of the new offset_table and size_table entries are only
> used by this command, you can avoid unnecessarily initializing
> everything in kernel_init() by doing something like this:
>
> static void
> diskio_option(void)
> {
> if (INVALID_MEMBER(class_devices)) {
> MEMBER_OFFSET_INIT(class_devices, "class", "class_devices");
> if (INVALID_MEMBER(class_devices))
> MEMBER_OFFSET_INIT(class_devices, "class", "devices");
> MEMBER_OFFSET_INIT(class_p, "class", "p");
> MEMBER_OFFSET_INIT(class_private_devices, "class_private",
> "class_devices");
> MEMBER_OFFSET_INIT(device_knode_class, "device", "knode_class");
> MEMBER_OFFSET_INIT(device_node, "device", "node");
> MEMBER_OFFSET_INIT(device_type, "device", "type");
> MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "dev");
> if (INVALID_MEMBER(gendisk_dev))
> MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "__dev");
> MEMBER_OFFSET_INIT(gendisk_kobj, "gendisk", "kobj");
> MEMBER_OFFSET_INIT(gendisk_part0, "gendisk", "part0");
> MEMBER_OFFSET_INIT(gendisk_queue, "gendisk", "queue");
> MEMBER_OFFSET_INIT(hd_struct_dev, "hd_struct", "__dev");
> MEMBER_OFFSET_INIT(klist_k_list, "klist", "k_list");
> MEMBER_OFFSET_INIT(klist_node_n_klist, "klist_node", "n_klist");
> MEMBER_OFFSET_INIT(klist_node_n_node, "klist_node", "n_node");
> MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
> MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> MEMBER_OFFSET_INIT(request_list_count, "request_list", "count");
> MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
> "in_flight");
> MEMBER_OFFSET_INIT(request_queue_rq, "request_queue", "rq");
> MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
> "klist_devices");
> MEMBER_OFFSET_INIT(subsystem_kset, "subsystem", "kset");
> STRUCT_SIZE_INIT(subsystem, "subsystem");
> STRUCT_SIZE_INIT(class_private, "class_private");
> MEMBER_SIZE_INIT(rq_in_flight, "request_queue", "in_flight");
> MEMBER_SIZE_INIT(class_private_devices, "class_private",
> "class_devices");
> }
>
> display_all_diskio();
> }

OK

>
> Secondly, the whole READ/WRITE issue is confusing to the uninitiated
> (like myself). After speaking with Jeff Moyer, we believe the output
> of your command could be clarified.
>
> Your help page indicates:
>
> +" This command dumps I/O statistics of all disks:",
> +" TOTAL: The total requests that have not been ended",
> +" READ: The total read requests that have not been ended",
> +" WRITE: The total write requests that have not been ended",
> +" DRV: The total requests that have been in the driver, but not end",
> +"",
> +" Note: some kernel does not contain read/write requests, and the command",
> +" will output '-----'"
> +"
EXAMPLES",
> +" %s> diskio",
> +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
> +NULL
>
> As Jeff explained to me, this is how it works:
>
> (1) In older kernels, this enum did not exist:
>
> enum {
> BLK_RW_ASYNC = 0,
> BLK_RW_SYNC = 1,
> };
>
> and in that case, the request_list.count[2] values are the
> READ/WRITE values, as you show in the help page example above.
>
> (2) In newer kernels, the enum does exist, and the meaning of the
> request_list.count[2] values are ASYNC/SYNC requests. In that
> case, you show "-----" under the READ and WRITE columns, which
> I found *very* confusing.
>
> What Jeff Moyer suggested is that -- in the case of new kernels -- you
> should alternatively show the counts with ASYNC and SYNC columns like
> this:
>
> MAJOR GENDISK NAME REQUEST QUEUE TOTAL ASYNC SYNC DRV",
> ...
>
> And the READ/WRITE vs. ASYNC/SYNC output display difference should be
> referenced in the help page output.

OK

>
> Third, a minor nit -- note that you misspelled it as "RUQUEST" both in
> the command and in the help page. Also, it appears that you used an IA64 as
> the example, given the GENDISK addresses. But aren't the REQUEST QUEUE
> addresses below beginning with "0x300..." user-space addresses for IA64?
>
> +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
>
> In any case, can you change it to use either x86_64 or x86 as an example?

OK

>
> Fourth, in testing this with a 2.6.25 kernel, the command hangs:
>
> crash> dev -d
> MAJOR GENDISK NAME REQUEST QUEUE TOTAL READ WRITE DRV
> 2 0xffff81012d8a5000 fd0 0xffff81012dc053c0 0 0 0 0
> 22 0xffff81012dc6b000 hdc 0xffff81012d8ae340 0 0 0 0
> 8 0xffff81012dd71000 sda 0xffff81012d8af040 0 0 0 0
> < hangs here forever >
>
> I'm not absolutely sure, but if I hit CTRL-C under gdb, it seems that it's
> spinning in next_disk_list() in that "goto again" loop?

I forgot to update list_head's address before 'goto again'. I will fix it.

>
> Fifth, I tried it on a much older RHEL3 kernel, which shows:
>
> crash> dev -d
> dev: invalid request_queue.in_flight's size
> crash>

I do not have such kernel. What is the version of such kernel?

>
> It's not really invalid size, but it's more the case that you
> don't support that old a kernel version. In that case, instead
> of doing this:
>
> error(FATAL, "invalid request_queue.in_flight's size
");
>
> you should do this instead:
>
> option_not_supported('d');

Good idea.

>
> And finally, whenever adding fields to the offset_table or size_table,
> please display their values in dump_offset_table() for the "help -o"
> command.

Sorry for forgetting it, and I will add it.

Thanks
Wen Congyang

>
> Thanks,
> Dave
>
>
>
>
>

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-30-2012, 04:32 AM
Wen Congyang
 
Default introduce a new command to display the disk's information

Hi, Dave

At 01/21/2012 04:24 AM, Dave Anderson Wrote:
>
>
> ----- Original Message -----
>> Hi, Dave
>>
>> When we investigate the problems of disk I/O, we want to get the disk's
>> gendisk address and request queue's address easily, and the requests num
>> is also important.
>>
>> Tha attached patch introduce a new command diskio to display such
>> information.
>>
>> Thanks
>> Wen Congyang
>
> Hello Wen,
>
> I built and tested this patch, and it certainly contains some useful
> information. However, I do have a several suggestions.
>
> First, there's no need to make it a unique command. It would be
> far more appropriate to make it a "dev" command option, which for
> example, already shows gendisk structure addresses for each of the
> block devices.
>
> So please move all of your functions from kernel.c to dev.c.
> Then, for example, use "dev -d", and have cmd_dev() call your
> command like this:
>
> while ((c = getopt(argcnt, args, "dpi")) != EOF) {
> switch(c)
> {
> case 'd':
> diskio_option();
> return;
> ...
>
> Since all of the new offset_table and size_table entries are only
> used by this command, you can avoid unnecessarily initializing
> everything in kernel_init() by doing something like this:
>
> static void
> diskio_option(void)
> {
> if (INVALID_MEMBER(class_devices)) {
> MEMBER_OFFSET_INIT(class_devices, "class", "class_devices");
> if (INVALID_MEMBER(class_devices))
> MEMBER_OFFSET_INIT(class_devices, "class", "devices");
> MEMBER_OFFSET_INIT(class_p, "class", "p");
> MEMBER_OFFSET_INIT(class_private_devices, "class_private",
> "class_devices");
> MEMBER_OFFSET_INIT(device_knode_class, "device", "knode_class");
> MEMBER_OFFSET_INIT(device_node, "device", "node");
> MEMBER_OFFSET_INIT(device_type, "device", "type");
> MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "dev");
> if (INVALID_MEMBER(gendisk_dev))
> MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "__dev");
> MEMBER_OFFSET_INIT(gendisk_kobj, "gendisk", "kobj");
> MEMBER_OFFSET_INIT(gendisk_part0, "gendisk", "part0");
> MEMBER_OFFSET_INIT(gendisk_queue, "gendisk", "queue");
> MEMBER_OFFSET_INIT(hd_struct_dev, "hd_struct", "__dev");
> MEMBER_OFFSET_INIT(klist_k_list, "klist", "k_list");
> MEMBER_OFFSET_INIT(klist_node_n_klist, "klist_node", "n_klist");
> MEMBER_OFFSET_INIT(klist_node_n_node, "klist_node", "n_node");
> MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
> MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> MEMBER_OFFSET_INIT(request_list_count, "request_list", "count");
> MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
> "in_flight");
> MEMBER_OFFSET_INIT(request_queue_rq, "request_queue", "rq");
> MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
> "klist_devices");
> MEMBER_OFFSET_INIT(subsystem_kset, "subsystem", "kset");
> STRUCT_SIZE_INIT(subsystem, "subsystem");
> STRUCT_SIZE_INIT(class_private, "class_private");
> MEMBER_SIZE_INIT(rq_in_flight, "request_queue", "in_flight");
> MEMBER_SIZE_INIT(class_private_devices, "class_private",
> "class_devices");
> }
>
> display_all_diskio();
> }
>
> Secondly, the whole READ/WRITE issue is confusing to the uninitiated
> (like myself). After speaking with Jeff Moyer, we believe the output
> of your command could be clarified.
>
> Your help page indicates:
>
> +" This command dumps I/O statistics of all disks:",
> +" TOTAL: The total requests that have not been ended",
> +" READ: The total read requests that have not been ended",
> +" WRITE: The total write requests that have not been ended",
> +" DRV: The total requests that have been in the driver, but not end",
> +"",
> +" Note: some kernel does not contain read/write requests, and the command",
> +" will output '-----'"
> +"
EXAMPLES",
> +" %s> diskio",
> +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
> +NULL
>
> As Jeff explained to me, this is how it works:
>
> (1) In older kernels, this enum did not exist:
>
> enum {
> BLK_RW_ASYNC = 0,
> BLK_RW_SYNC = 1,
> };
>
> and in that case, the request_list.count[2] values are the
> READ/WRITE values, as you show in the help page example above.
>
> (2) In newer kernels, the enum does exist, and the meaning of the
> request_list.count[2] values are ASYNC/SYNC requests. In that
> case, you show "-----" under the READ and WRITE columns, which
> I found *very* confusing.
>
> What Jeff Moyer suggested is that -- in the case of new kernels -- you
> should alternatively show the counts with ASYNC and SYNC columns like
> this:
>
> MAJOR GENDISK NAME REQUEST QUEUE TOTAL ASYNC SYNC DRV",
> ...
>
> And the READ/WRITE vs. ASYNC/SYNC output display difference should be
> referenced in the help page output.
>
> Third, a minor nit -- note that you misspelled it as "RUQUEST" both in
> the command and in the help page. Also, it appears that you used an IA64 as
> the example, given the GENDISK addresses. But aren't the REQUEST QUEUE
> addresses below beginning with "0x300..." user-space addresses for IA64?
>
> +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
>
> In any case, can you change it to use either x86_64 or x86 as an example?
>
> Fourth, in testing this with a 2.6.25 kernel, the command hangs:
>
> crash> dev -d
> MAJOR GENDISK NAME REQUEST QUEUE TOTAL READ WRITE DRV
> 2 0xffff81012d8a5000 fd0 0xffff81012dc053c0 0 0 0 0
> 22 0xffff81012dc6b000 hdc 0xffff81012d8ae340 0 0 0 0
> 8 0xffff81012dd71000 sda 0xffff81012d8af040 0 0 0 0
> < hangs here forever >
>
> I'm not absolutely sure, but if I hit CTRL-C under gdb, it seems that it's
> spinning in next_disk_list() in that "goto again" loop?
>
> Fifth, I tried it on a much older RHEL3 kernel, which shows:
>
> crash> dev -d
> dev: invalid request_queue.in_flight's size
> crash>
>
> It's not really invalid size, but it's more the case that you
> don't support that old a kernel version. In that case, instead
> of doing this:
>
> error(FATAL, "invalid request_queue.in_flight's size
");
>
> you should do this instead:
>
> option_not_supported('d');
>
> And finally, whenever adding fields to the offset_table or size_table,
> please display their values in dump_offset_table() for the "help -o"
> command.

I have updated the patch.

Thanks
Wen Congyang

>
> Thanks,
> Dave
>
>
>
>
>

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-30-2012, 07:49 PM
Dave Anderson
 
Default introduce a new command to display the disk's information

----- Original Message -----
> Hi, Dave
>
> At 01/21/2012 04:24 AM, Dave Anderson Wrote:
> >
> >
> > ----- Original Message -----
> >> Hi, Dave
> >>
> >> When we investigate the problems of disk I/O, we want to get the disk's
> >> gendisk address and request queue's address easily, and the requests num
> >> is also important.
> >>
> >> Tha attached patch introduce a new command diskio to display such information.
> >>
> >> Thanks
> >> Wen Congyang
> >
> > Hello Wen,
> >
> > I built and tested this patch, and it certainly contains some useful
> > information. However, I do have a several suggestions.
> >
> > First, there's no need to make it a unique command. It would be
> > far more appropriate to make it a "dev" command option, which for
> > example, already shows gendisk structure addresses for each of the
> > block devices.
> >
> > So please move all of your functions from kernel.c to dev.c.
> > Then, for example, use "dev -d", and have cmd_dev() call your
> > command like this:
> >
> > while ((c = getopt(argcnt, args, "dpi")) != EOF) {
> > switch(c)
> > {
> > case 'd':
> > diskio_option();
> > return;
> > ...
> >
> > Since all of the new offset_table and size_table entries are only
> > used by this command, you can avoid unnecessarily initializing
> > everything in kernel_init() by doing something like this:
> >
> > static void
> > diskio_option(void)
> > {
> > if (INVALID_MEMBER(class_devices)) {
> > MEMBER_OFFSET_INIT(class_devices, "class", "class_devices");
> > if (INVALID_MEMBER(class_devices))
> > MEMBER_OFFSET_INIT(class_devices, "class", "devices");
> > MEMBER_OFFSET_INIT(class_p, "class", "p");
> > MEMBER_OFFSET_INIT(class_private_devices, "class_private",
> > "class_devices");
> > MEMBER_OFFSET_INIT(device_knode_class, "device", "knode_class");
> > MEMBER_OFFSET_INIT(device_node, "device", "node");
> > MEMBER_OFFSET_INIT(device_type, "device", "type");
> > MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "dev");
> > if (INVALID_MEMBER(gendisk_dev))
> > MEMBER_OFFSET_INIT(gendisk_dev, "gendisk", "__dev");
> > MEMBER_OFFSET_INIT(gendisk_kobj, "gendisk", "kobj");
> > MEMBER_OFFSET_INIT(gendisk_part0, "gendisk", "part0");
> > MEMBER_OFFSET_INIT(gendisk_queue, "gendisk", "queue");
> > MEMBER_OFFSET_INIT(hd_struct_dev, "hd_struct", "__dev");
> > MEMBER_OFFSET_INIT(klist_k_list, "klist", "k_list");
> > MEMBER_OFFSET_INIT(klist_node_n_klist, "klist_node", "n_klist");
> > MEMBER_OFFSET_INIT(klist_node_n_node, "klist_node", "n_node");
> > MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
> > MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> > MEMBER_OFFSET_INIT(request_list_count, "request_list", "count");
> > MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
> > "in_flight");
> > MEMBER_OFFSET_INIT(request_queue_rq, "request_queue", "rq");
> > MEMBER_OFFSET_INIT(subsys_private_klist_devices,
> > "subsys_private",
> > "klist_devices");
> > MEMBER_OFFSET_INIT(subsystem_kset, "subsystem", "kset");
> > STRUCT_SIZE_INIT(subsystem, "subsystem");
> > STRUCT_SIZE_INIT(class_private, "class_private");
> > MEMBER_SIZE_INIT(rq_in_flight, "request_queue", "in_flight");
> > MEMBER_SIZE_INIT(class_private_devices, "class_private",
> > "class_devices");
> > }
> >
> > display_all_diskio();
> > }
> >
> > Secondly, the whole READ/WRITE issue is confusing to the uninitiated
> > (like myself). After speaking with Jeff Moyer, we believe the output
> > of your command could be clarified.
> >
> > Your help page indicates:
> >
> > +" This command dumps I/O statistics of all disks:",
> > +" TOTAL: The total requests that have not been ended",
> > +" READ: The total read requests that have not been ended",
> > +" WRITE: The total write requests that have not been ended",
> > +" DRV: The total requests that have been in the driver, but not end",
> > +"",
> > +" Note: some kernel does not contain read/write requests, and the command",
> > +" will output '-----'"
> > +"
EXAMPLES",
> > +" %s> diskio",
> > +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> > +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> > +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> > +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> > +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> > +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
> > +NULL
> >
> > As Jeff explained to me, this is how it works:
> >
> > (1) In older kernels, this enum did not exist:
> >
> > enum {
> > BLK_RW_ASYNC = 0,
> > BLK_RW_SYNC = 1,
> > };
> >
> > and in that case, the request_list.count[2] values are the
> > READ/WRITE values, as you show in the help page example above.
> >
> > (2) In newer kernels, the enum does exist, and the meaning of the
> > request_list.count[2] values are ASYNC/SYNC requests. In that
> > case, you show "-----" under the READ and WRITE columns, which
> > I found *very* confusing.
> >
> > What Jeff Moyer suggested is that -- in the case of new kernels -- you
> > should alternatively show the counts with ASYNC and SYNC columns like
> > this:
> >
> > MAJOR GENDISK NAME REQUEST QUEUE TOTAL ASYNC SYNC DRV",
> > ...
> >
> > And the READ/WRITE vs. ASYNC/SYNC output display difference should be
> > referenced in the help page output.
> >
> > Third, a minor nit -- note that you misspelled it as "RUQUEST" both in
> > the command and in the help page. Also, it appears that you used an IA64 as
> > the example, given the GENDISK addresses. But aren't the REQUEST QUEUE
> > addresses below beginning with "0x300..." user-space addresses for IA64?
> >
> > +" MAJOR GENDISK NAME RUQUEST QUEUE TOTAL READ WRITE DRV",
> > +" 008 0xe00000010773ea80 sda 0x3000000109c9fbf0 12 0 12 0",
> > +" 008 0xe00000010773e680 sdb 0x3000000109c9f8a0 2 2 0 0",
> > +" 008 0xe000000107781d80 sdc 0x300000010c268050 6 0 6 6",
> > +" 008 0xe00000010773e080 sdd 0x300000010c26bbf0 0 0 0 0",
> > +" 008 0xe00000010773dc80 sde 0x300000010c3dd780 0 0 0 0",
> >
> > In any case, can you change it to use either x86_64 or x86 as an
> > example?
> >
> > Fourth, in testing this with a 2.6.25 kernel, the command hangs:
> >
> > crash> dev -d
> > MAJOR GENDISK NAME REQUEST QUEUE TOTAL READ WRITE DRV
> > 2 0xffff81012d8a5000 fd0 0xffff81012dc053c0 0 0 0 0
> > 22 0xffff81012dc6b000 hdc 0xffff81012d8ae340 0 0 0 0
> > 8 0xffff81012dd71000 sda 0xffff81012d8af040 0 0 0 0
> > < hangs here forever >
> >
> > I'm not absolutely sure, but if I hit CTRL-C under gdb, it seems that it's
> > spinning in next_disk_list() in that "goto again" loop?
> >
> > Fifth, I tried it on a much older RHEL3 kernel, which shows:
> >
> > crash> dev -d
> > dev: invalid request_queue.in_flight's size
> > crash>
> >
> > It's not really invalid size, but it's more the case that you
> > don't support that old a kernel version. In that case, instead
> > of doing this:
> >
> > error(FATAL, "invalid request_queue.in_flight's size
");
> >
> > you should do this instead:
> >
> > option_not_supported('d');
> >
> > And finally, whenever adding fields to the offset_table or size_table,
> > please display their values in dump_offset_table() for the "help -o"
> > command.
>
> I have updated the patch.
>
> Thanks
> Wen Congyang

Hello Wen,

This second patch looks good -- I did make a few small changes.
I re-worded the help page:

crash> help dev

NAME
dev - device data

SYNOPSIS
dev [-i | -p | -d]

DESCRIPTION
If no argument is entered, this command dumps character and block
device data.

-i display I/O port usage; on 2.4 kernels, also display I/O memory usage.
-p display PCI device data.
-d display disk I/O statistics:
TOTAL: total number of allocated I/O requests that are in-progress
SYNC: I/O requests that are synchronous
ASYNC: I/O requests that are asynchronous
READ: I/O requests that are reads (older kernels)
WRITE: I/O requests that are writes (older kernels)
DRV: I/O requests that are in-flight in the device driver

EXAMPLES
...

I reformatted the output to allow for larger disk name strings,
such as "cciss/c0d0" in this example:

crash> dev -d
MAJOR GENDISK NAME REQUEST QUEUE TOTAL READ WRITE DRV
104 0xffff81007e422800 cciss/c0d0 0xffff81007f6d4048 1 0 1 1
253 0xffff81007e89fa00 dm-0 0xffff81007f6d4c98 0 0 0 0
253 0xffff81007eace200 dm-1 0xffff81007f6d4670 0 0 0 0
9 0xffff81007eb43400 md0 0xffff81007d397928 0 0 0 0
crash>

And I created a dev_table.flags DISKIO_INIT bit instead of using
the static "initialized" variable.

Queued for crash-6.0.3.

Thanks,
Dave





--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
 
Old 01-30-2012, 07:56 PM
Dave Anderson
 
Default introduce a new command to display the disk's information

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

> >
> > Fifth, I tried it on a much older RHEL3 kernel, which shows:
> >
> > crash> dev -d
> > dev: invalid request_queue.in_flight's size
> > crash>
>
> I do not have such kernel. What is the version of such kernel?
> >
> > It's not really invalid size, but it's more the case that you
> > don't support that old a kernel version. In that case, instead
> > of doing this:
> >
> > error(FATAL, "invalid request_queue.in_flight's size
");
> >
> > you should do this instead:
> >
> > option_not_supported('d');
>
> Good idea.

RHEL3 was a 2.4.21-based kernel, where the request_queue didn't have
the in_flight member. So the use of the option_not_supported() is
good enough.

Dave


--
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 06:46 PM.

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