| Message ID | 20190701215726.27601-4-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: udpate debug messages with blk_op_str() | expand |
> diff --git a/block/blk-core.c b/block/blk-core.c > index 5143a8e19b63..9855c5d5027d 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1127,17 +1127,15 @@ EXPORT_SYMBOL_GPL(direct_make_request); > */ > blk_qc_t submit_bio(struct bio *bio) > { > + unsigned int count = bio_sectors(bio); Chaitanya, Could it have a single empty line right after this just like you have for the if-statement below for the block_dump. It's just a nitpick. > /* > * If it's a regular read/write or a barrier with data attached, > * go through the normal accounting stuff before submission. > */ > if (bio_has_data(bio)) { > - unsigned int count; > > if (unlikely(bio_op(bio) == REQ_OP_WRITE_SAME)) > count = queue_logical_block_size(bio->bi_disk->queue) >> 9; > - else > - count = bio_sectors(bio); > > if (op_is_write(bio_op(bio))) { > count_vm_events(PGPGOUT, count); > @@ -1145,15 +1143,16 @@ blk_qc_t submit_bio(struct bio *bio) > task_io_account_read(bio->bi_iter.bi_size); > count_vm_events(PGPGIN, count); > } > + } > > - if (unlikely(block_dump)) { > - char b[BDEVNAME_SIZE]; > - printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", > - current->comm, task_pid_nr(current), > - blk_op_str(bio_op(bio)), > - (unsigned long long)bio->bi_iter.bi_sector, > - bio_devname(bio, b), count); > - } > + if (unlikely(block_dump)) { > + char b[BDEVNAME_SIZE]; > + > + printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", > + current->comm, task_pid_nr(current), > + blk_op_str(bio_op(bio)), > + (unsigned long long)bio->bi_iter.bi_sector, > + bio_devname(bio, b), count); It would be great if non-data command is traced, I think. Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
On 7/2/19 5:50 PM, Minwoo Im wrote: >> diff --git a/block/blk-core.c b/block/blk-core.c >> index 5143a8e19b63..9855c5d5027d 100644 >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -1127,17 +1127,15 @@ EXPORT_SYMBOL_GPL(direct_make_request); >> */ >> blk_qc_t submit_bio(struct bio *bio) >> { >> + unsigned int count = bio_sectors(bio); > Chaitanya, > > Could it have a single empty line right after this just like you have > for the if-statement below for the block_dump. It's just a nitpick. Yeah, again can be done at the time of applying patch, if Jens is okay with that. Otherwise I can send V2. > >> /* >> * If it's a regular read/write or a barrier with data attached, >> * go through the normal accounting stuff before submission. >> */ >> if (bio_has_data(bio)) { >> - unsigned int count; >> >> if (unlikely(bio_op(bio) == REQ_OP_WRITE_SAME)) >> count = queue_logical_block_size(bio->bi_disk->queue) >> 9; >> - else >> - count = bio_sectors(bio); >> >> if (op_is_write(bio_op(bio))) { >> count_vm_events(PGPGOUT, count); >> @@ -1145,15 +1143,16 @@ blk_qc_t submit_bio(struct bio *bio) >> task_io_account_read(bio->bi_iter.bi_size); >> count_vm_events(PGPGIN, count); >> } >> + } >> >> - if (unlikely(block_dump)) { >> - char b[BDEVNAME_SIZE]; >> - printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", >> - current->comm, task_pid_nr(current), >> - blk_op_str(bio_op(bio)), >> - (unsigned long long)bio->bi_iter.bi_sector, >> - bio_devname(bio, b), count); >> - } >> + if (unlikely(block_dump)) { >> + char b[BDEVNAME_SIZE]; >> + >> + printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", >> + current->comm, task_pid_nr(current), >> + blk_op_str(bio_op(bio)), >> + (unsigned long long)bio->bi_iter.bi_sector, >> + bio_devname(bio, b), count); > It would be great if non-data command is traced, I think. > > Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com> >
diff --git a/block/blk-core.c b/block/blk-core.c index 5143a8e19b63..9855c5d5027d 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1127,17 +1127,15 @@ EXPORT_SYMBOL_GPL(direct_make_request); */ blk_qc_t submit_bio(struct bio *bio) { + unsigned int count = bio_sectors(bio); /* * If it's a regular read/write or a barrier with data attached, * go through the normal accounting stuff before submission. */ if (bio_has_data(bio)) { - unsigned int count; if (unlikely(bio_op(bio) == REQ_OP_WRITE_SAME)) count = queue_logical_block_size(bio->bi_disk->queue) >> 9; - else - count = bio_sectors(bio); if (op_is_write(bio_op(bio))) { count_vm_events(PGPGOUT, count); @@ -1145,15 +1143,16 @@ blk_qc_t submit_bio(struct bio *bio) task_io_account_read(bio->bi_iter.bi_size); count_vm_events(PGPGIN, count); } + } - if (unlikely(block_dump)) { - char b[BDEVNAME_SIZE]; - printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", - current->comm, task_pid_nr(current), - blk_op_str(bio_op(bio)), - (unsigned long long)bio->bi_iter.bi_sector, - bio_devname(bio, b), count); - } + if (unlikely(block_dump)) { + char b[BDEVNAME_SIZE]; + + printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n", + current->comm, task_pid_nr(current), + blk_op_str(bio_op(bio)), + (unsigned long long)bio->bi_iter.bi_sector, + bio_devname(bio, b), count); } return generic_make_request(bio);
In the current implementation when block_dump is enabled we only report bios with data. In this way we are not logging the REQ_OP_WRITE_ZEROES, REQ_OP_DISCARD or any other operations without data etc. This patch allows all bios with and without data to be reported when block_dump is enabled and adjust the existing code. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- --- block/blk-core.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)