linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] block: udpate debug messages with blk_op_str()
@ 2019-07-01 21:57 Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 1/5] block: update error message for bio_check_ro() Chaitanya Kulkarni
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

Hi,

This patch-series uses newly introduced blk_op_str() to improve
existing REQ_OP_XXX messages. The first two patches we change the
bio_check_ro() and submit_bio() and make debugging more clear and
get rid of the 1:M mapping of the REQ_OP_XXX to debug string
(such as printing "READ" and "WRITE") with the help of blk_op_str().

Later 3 patches are focused on changing the block_dump in submit_bio(),
so we can log all the operations and update the respective
documentation.

This is needed as we are adding more REQ_OP_XXX with last bit set 
as a part of newly introduced Zone Block Device Zone Open, Zone Close
and Zone Finish operations which are mapped to new REQ_OP_ZONE_OPEN,
REQ_OP_ZONE_CLOSE and REQ_OP_ZONE_FINISH respectively [1].

With this patch-series, we can see the following output with respective
commands which are clear including the special REQ_OP_XXX
(Write-zeroes and Discard) :-

# blkzone reset /dev/nullb0			# Reset all the zones 
# blkdiscard -o 0 -l 4096 /dev/nullb0		# discard 8 sectors 
# blkdiscard -o 0 -l 40960 /dev/nullb0		# disacrd 80 sectors  
# blkdiscard -z -o 0 -l 40960 /dev/nullb0	# write-zero 80 sectors
# dmesg  -c 

<snip>
[ 1161.922707] blkzone(10803): ZONE_RESET block 0 on nullb0 (0 sectors)
[ 1161.922735] blkzone(10803): ZONE_RESET block 524288 on nullb0 (0 sectors)
[ 1161.922750] blkzone(10803): ZONE_RESET block 1048576 on nullb0 (0 sectors)
[ 1161.922762] blkzone(10803): ZONE_RESET block 1572864 on nullb0 (0 sectors)
[ 1186.949689] blkdiscard(10834): DISCARD block 0 on nullb0 (8 sectors)
[ 1195.145731] blkdiscard(10844): DISCARD block 0 on nullb0 (80 sectors)
[ 1212.490633] blkdiscard(10854): WRITE_ZEROES block 0 on nullb0 (80 sectors)
<snip>

Regards,
Chaitanya

To: linux-mm@kvack.org
To; linux-block@ linux-block@vger.kernel.org
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Jenx Axboe <axboe@kernel.dk>

[1] https://www.spinics.net/lists/linux-block/msg41884.html.

Chaitanya Kulkarni (5):
  block: update error message for bio_check_ro()
  block: update error message in submit_bio()
  block: allow block_dump to print all REQ_OP_XXX
  mm: update block_dump comment
  Documentation/laptop: add block_dump documentation

 Documentation/laptops/laptop-mode.txt | 16 ++++++++--------
 block/blk-core.c                      | 27 +++++++++++++--------------
 mm/page-writeback.c                   |  2 +-
 3 files changed, 22 insertions(+), 23 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] block: update error message for bio_check_ro()
  2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
@ 2019-07-01 21:57 ` Chaitanya Kulkarni
  2019-07-03  0:42   ` Minwoo Im
  2019-07-01 21:57 ` [PATCH 2/5] block: update error message in submit_bio() Chaitanya Kulkarni
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

The existing code in the bio_check_ro() relies on the op_is_write().
op_is_write() checks for the last bit in the bio_op(). Now that we have
multiple REQ_OP_XXX with last bit set to 1 such as, (from blk_types.h):

	/* write sectors to the device */
	REQ_OP_WRITE		= 1,
	/* flush the volatile write cache */
	REQ_OP_DISCARD		= 3,
	/* securely erase sectors */
	REQ_OP_SECURE_ERASE	= 5,
	/* write the same sector many times */
	REQ_OP_WRITE_SAME	= 7,
	/* write the zero filled sector many times */
	REQ_OP_WRITE_ZEROES	= 9,

it is hard to understand which bio op failed in the bio_check_ro().

Modify the error message in bio_check_ro() to print correct REQ_OP_XXX
with the help of blk_op_str().

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/blk-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 5d1fc8e17dd1..47c8b9c48a57 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -786,9 +786,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)
 			return false;
 
 		WARN_ONCE(1,
-		       "generic_make_request: Trying to write "
-			"to read-only block-device %s (partno %d)\n",
-			bio_devname(bio, b), part->partno);
+			"generic_make_request: Trying op %s on the "
+			"read-only block-device %s (partno %d)\n",
+			blk_op_str(op), bio_devname(bio, b), part->partno);
 		/* Older lvm-tools actually trigger this */
 		return false;
 	}
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] block: update error message in submit_bio()
  2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 1/5] block: update error message for bio_check_ro() Chaitanya Kulkarni
@ 2019-07-01 21:57 ` Chaitanya Kulkarni
  2019-07-03  0:43   ` Minwoo Im
  2019-07-01 21:57 ` [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

The existing code in the submit_bio() relies on the op_is_write().
op_is_write() checks for the last bit in the bio_op() and we only
print WRITE or READ as a bio_op().

It is hard to understand which bio op based on READ/WRITE in
submit_bio() with addition of newly discussed REQ_OP_XXX. [1]

Modify the error message in submit_bio() to print correct REQ_OP_XXX
with the help of blk_op_str().

[1] https://www.spinics.net/lists/linux-block/msg41884.html. 

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 block/blk-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 47c8b9c48a57..5143a8e19b63 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1150,7 +1150,7 @@ blk_qc_t submit_bio(struct bio *bio)
 			char b[BDEVNAME_SIZE];
 			printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n",
 			current->comm, task_pid_nr(current),
-				op_is_write(bio_op(bio)) ? "WRITE" : "READ",
+				blk_op_str(bio_op(bio)),
 				(unsigned long long)bio->bi_iter.bi_sector,
 				bio_devname(bio, b), count);
 		}
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX
  2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 1/5] block: update error message for bio_check_ro() Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 2/5] block: update error message in submit_bio() Chaitanya Kulkarni
@ 2019-07-01 21:57 ` Chaitanya Kulkarni
  2019-07-03  0:50   ` Minwoo Im
  2019-07-01 21:57 ` [PATCH 4/5] mm: update block_dump comment Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 5/5] Documentation/laptop: add block_dump documentation Chaitanya Kulkarni
  4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

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(-)

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);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] mm: update block_dump comment
  2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
                   ` (2 preceding siblings ...)
  2019-07-01 21:57 ` [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX Chaitanya Kulkarni
@ 2019-07-01 21:57 ` Chaitanya Kulkarni
  2019-07-08 18:49   ` Chaitanya Kulkarni
  2019-07-01 21:57 ` [PATCH 5/5] Documentation/laptop: add block_dump documentation Chaitanya Kulkarni
  4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

With respect to the changes in the submit_bio() in the earlier patch
now we report all the REQ_OP_XXX associated with bio along with
REQ_OP_READ and REQ_OP_WRITE (READ/WRITE). Update the following
comment for block_dump variable to reflect the change.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 mm/page-writeback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index bdbe8b6b1225..ef299f95349f 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -109,7 +109,7 @@ EXPORT_SYMBOL_GPL(dirty_writeback_interval);
 unsigned int dirty_expire_interval = 30 * 100; /* centiseconds */
 
 /*
- * Flag that makes the machine dump writes/reads and block dirtyings.
+ * Flag that makes the machine dump block layer requests and block dirtyings.
  */
 int block_dump;
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] Documentation/laptop: add block_dump documentation
  2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
                   ` (3 preceding siblings ...)
  2019-07-01 21:57 ` [PATCH 4/5] mm: update block_dump comment Chaitanya Kulkarni
@ 2019-07-01 21:57 ` Chaitanya Kulkarni
  2019-07-08 18:49   ` Chaitanya Kulkarni
  4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-01 21:57 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe, Chaitanya Kulkarni

This patch updates the block_dump documentation with respect to the
changes from the earlier patch for submit_bio(). Also we adjust rest of
the lines to fit with standaed format.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 Documentation/laptops/laptop-mode.txt | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/laptops/laptop-mode.txt b/Documentation/laptops/laptop-mode.txt
index 1c707fc9b141..d4d72ed677c4 100644
--- a/Documentation/laptops/laptop-mode.txt
+++ b/Documentation/laptops/laptop-mode.txt
@@ -101,14 +101,14 @@ a cache miss. The disk can then be spun down in the periods of inactivity.
 
 If you want to find out which process caused the disk to spin up, you can
 gather information by setting the flag /proc/sys/vm/block_dump. When this flag
-is set, Linux reports all disk read and write operations that take place, and
-all block dirtyings done to files. This makes it possible to debug why a disk
-needs to spin up, and to increase battery life even more. The output of
-block_dump is written to the kernel output, and it can be retrieved using
-"dmesg". When you use block_dump and your kernel logging level also includes
-kernel debugging messages, you probably want to turn off klogd, otherwise
-the output of block_dump will be logged, causing disk activity that is not
-normally there.
+is set, Linux reports all disk I/O operations along with read and write
+operations that take place, and all block dirtyings done to files. This makes
+it possible to debug why a disk needs to spin up, and to increase battery life
+even more. The output of block_dump is written to the kernel output, and it can
+be retrieved using "dmesg". When you use block_dump and your kernel logging
+level also includes kernel debugging messages, you probably want to turn off
+klogd, otherwise the output of block_dump will be logged, causing disk activity
+that is not normally there.
 
 
 Configuration
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] block: update error message for bio_check_ro()
  2019-07-01 21:57 ` [PATCH 1/5] block: update error message for bio_check_ro() Chaitanya Kulkarni
@ 2019-07-03  0:42   ` Minwoo Im
  2019-07-03  2:24     ` Chaitanya Kulkarni
  0 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2019-07-03  0:42 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-mm, linux-block, bvanassche, axboe, Minwoo Im

On 19-07-01 14:57:22, Chaitanya Kulkarni wrote:
> The existing code in the bio_check_ro() relies on the op_is_write().
> op_is_write() checks for the last bit in the bio_op(). Now that we have
> multiple REQ_OP_XXX with last bit set to 1 such as, (from blk_types.h):
> 
> 	/* write sectors to the device */
> 	REQ_OP_WRITE		= 1,
> 	/* flush the volatile write cache */
> 	REQ_OP_DISCARD		= 3,
> 	/* securely erase sectors */
> 	REQ_OP_SECURE_ERASE	= 5,
> 	/* write the same sector many times */
> 	REQ_OP_WRITE_SAME	= 7,
> 	/* write the zero filled sector many times */
> 	REQ_OP_WRITE_ZEROES	= 9,
> 
> it is hard to understand which bio op failed in the bio_check_ro().
> 
> Modify the error message in bio_check_ro() to print correct REQ_OP_XXX
> with the help of blk_op_str().
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  block/blk-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 5d1fc8e17dd1..47c8b9c48a57 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -786,9 +786,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)
>  			return false;
>  
>  		WARN_ONCE(1,
> -		       "generic_make_request: Trying to write "
> -			"to read-only block-device %s (partno %d)\n",
> -			bio_devname(bio, b), part->partno);
> +			"generic_make_request: Trying op %s on the "
> +			"read-only block-device %s (partno %d)\n",
> +			blk_op_str(op), bio_devname(bio, b), part->partno);

Maybe "s/Trying op %s on/Tyring op %s to" just like the previous one?
Not a native speaker, though ;)

I think it would be better to see the log which holds the exact request
operation type in a string.

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] block: update error message in submit_bio()
  2019-07-01 21:57 ` [PATCH 2/5] block: update error message in submit_bio() Chaitanya Kulkarni
@ 2019-07-03  0:43   ` Minwoo Im
  0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2019-07-03  0:43 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-mm, linux-block, bvanassche, axboe, Minwoo Im

On 19-07-01 14:57:23, Chaitanya Kulkarni wrote:
> The existing code in the submit_bio() relies on the op_is_write().
> op_is_write() checks for the last bit in the bio_op() and we only
> print WRITE or READ as a bio_op().
> 
> It is hard to understand which bio op based on READ/WRITE in
> submit_bio() with addition of newly discussed REQ_OP_XXX. [1]
> 
> Modify the error message in submit_bio() to print correct REQ_OP_XXX
> with the help of blk_op_str().
> 
> [1] https://www.spinics.net/lists/linux-block/msg41884.html. 
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

It looks good to me.

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX
  2019-07-01 21:57 ` [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX Chaitanya Kulkarni
@ 2019-07-03  0:50   ` Minwoo Im
  2019-07-03  2:26     ` Chaitanya Kulkarni
  0 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2019-07-03  0:50 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: linux-mm, linux-block, bvanassche, axboe, Minwoo Im

> 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>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] block: update error message for bio_check_ro()
  2019-07-03  0:42   ` Minwoo Im
@ 2019-07-03  2:24     ` Chaitanya Kulkarni
  0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-03  2:24 UTC (permalink / raw)
  To: Minwoo Im; +Cc: linux-mm, linux-block, bvanassche, axboe

On 7/2/19 5:42 PM, Minwoo Im wrote:
> On 19-07-01 14:57:22, Chaitanya Kulkarni wrote:
>> The existing code in the bio_check_ro() relies on the op_is_write().
>> op_is_write() checks for the last bit in the bio_op(). Now that we have
>> multiple REQ_OP_XXX with last bit set to 1 such as, (from blk_types.h):
>>
>> 	/* write sectors to the device */
>> 	REQ_OP_WRITE		= 1,
>> 	/* flush the volatile write cache */
>> 	REQ_OP_DISCARD		= 3,
>> 	/* securely erase sectors */
>> 	REQ_OP_SECURE_ERASE	= 5,
>> 	/* write the same sector many times */
>> 	REQ_OP_WRITE_SAME	= 7,
>> 	/* write the zero filled sector many times */
>> 	REQ_OP_WRITE_ZEROES	= 9,
>>
>> it is hard to understand which bio op failed in the bio_check_ro().
>>
>> Modify the error message in bio_check_ro() to print correct REQ_OP_XXX
>> with the help of blk_op_str().
>>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
>> ---
>>  block/blk-core.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 5d1fc8e17dd1..47c8b9c48a57 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -786,9 +786,9 @@ static inline bool bio_check_ro(struct bio *bio, struct hd_struct *part)
>>  			return false;
>>  
>>  		WARN_ONCE(1,
>> -		       "generic_make_request: Trying to write "
>> -			"to read-only block-device %s (partno %d)\n",
>> -			bio_devname(bio, b), part->partno);
>> +			"generic_make_request: Trying op %s on the "
>> +			"read-only block-device %s (partno %d)\n",
>> +			blk_op_str(op), bio_devname(bio, b), part->partno);
> Maybe "s/Trying op %s on/Tyring op %s to" just like the previous one?
> Not a native speaker, though ;)

Yeah, can be done at the time of applying patch if Jens is okay with it.

Otherwise I'll send V2.

>
> I think it would be better to see the log which holds the exact request
> operation type in a string.
>
> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX
  2019-07-03  0:50   ` Minwoo Im
@ 2019-07-03  2:26     ` Chaitanya Kulkarni
  0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-03  2:26 UTC (permalink / raw)
  To: Minwoo Im; +Cc: linux-mm, linux-block, bvanassche, axboe

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>
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/5] mm: update block_dump comment
  2019-07-01 21:57 ` [PATCH 4/5] mm: update block_dump comment Chaitanya Kulkarni
@ 2019-07-08 18:49   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-08 18:49 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe

Can someone from linux-mm list please provide a feedback on this ?

On 07/01/2019 02:58 PM, Chaitanya Kulkarni wrote:
> With respect to the changes in the submit_bio() in the earlier patch
> now we report all the REQ_OP_XXX associated with bio along with
> REQ_OP_READ and REQ_OP_WRITE (READ/WRITE). Update the following
> comment for block_dump variable to reflect the change.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>   mm/page-writeback.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index bdbe8b6b1225..ef299f95349f 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -109,7 +109,7 @@ EXPORT_SYMBOL_GPL(dirty_writeback_interval);
>   unsigned int dirty_expire_interval = 30 * 100; /* centiseconds */
>
>   /*
> - * Flag that makes the machine dump writes/reads and block dirtyings.
> + * Flag that makes the machine dump block layer requests and block dirtyings.
>    */
>   int block_dump;
>
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] Documentation/laptop: add block_dump documentation
  2019-07-01 21:57 ` [PATCH 5/5] Documentation/laptop: add block_dump documentation Chaitanya Kulkarni
@ 2019-07-08 18:49   ` Chaitanya Kulkarni
  0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2019-07-08 18:49 UTC (permalink / raw)
  To: linux-mm, linux-block; +Cc: bvanassche, axboe

Can someone from linux-mm list please provide a feedback on this ?

On 07/01/2019 02:58 PM, Chaitanya Kulkarni wrote:
> This patch updates the block_dump documentation with respect to the
> changes from the earlier patch for submit_bio(). Also we adjust rest of
> the lines to fit with standaed format.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>   Documentation/laptops/laptop-mode.txt | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/laptops/laptop-mode.txt b/Documentation/laptops/laptop-mode.txt
> index 1c707fc9b141..d4d72ed677c4 100644
> --- a/Documentation/laptops/laptop-mode.txt
> +++ b/Documentation/laptops/laptop-mode.txt
> @@ -101,14 +101,14 @@ a cache miss. The disk can then be spun down in the periods of inactivity.
>
>   If you want to find out which process caused the disk to spin up, you can
>   gather information by setting the flag /proc/sys/vm/block_dump. When this flag
> -is set, Linux reports all disk read and write operations that take place, and
> -all block dirtyings done to files. This makes it possible to debug why a disk
> -needs to spin up, and to increase battery life even more. The output of
> -block_dump is written to the kernel output, and it can be retrieved using
> -"dmesg". When you use block_dump and your kernel logging level also includes
> -kernel debugging messages, you probably want to turn off klogd, otherwise
> -the output of block_dump will be logged, causing disk activity that is not
> -normally there.
> +is set, Linux reports all disk I/O operations along with read and write
> +operations that take place, and all block dirtyings done to files. This makes
> +it possible to debug why a disk needs to spin up, and to increase battery life
> +even more. The output of block_dump is written to the kernel output, and it can
> +be retrieved using "dmesg". When you use block_dump and your kernel logging
> +level also includes kernel debugging messages, you probably want to turn off
> +klogd, otherwise the output of block_dump will be logged, causing disk activity
> +that is not normally there.
>
>
>   Configuration
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-07-08 18:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01 21:57 [PATCH 0/5] block: udpate debug messages with blk_op_str() Chaitanya Kulkarni
2019-07-01 21:57 ` [PATCH 1/5] block: update error message for bio_check_ro() Chaitanya Kulkarni
2019-07-03  0:42   ` Minwoo Im
2019-07-03  2:24     ` Chaitanya Kulkarni
2019-07-01 21:57 ` [PATCH 2/5] block: update error message in submit_bio() Chaitanya Kulkarni
2019-07-03  0:43   ` Minwoo Im
2019-07-01 21:57 ` [PATCH 3/5] block: allow block_dump to print all REQ_OP_XXX Chaitanya Kulkarni
2019-07-03  0:50   ` Minwoo Im
2019-07-03  2:26     ` Chaitanya Kulkarni
2019-07-01 21:57 ` [PATCH 4/5] mm: update block_dump comment Chaitanya Kulkarni
2019-07-08 18:49   ` Chaitanya Kulkarni
2019-07-01 21:57 ` [PATCH 5/5] Documentation/laptop: add block_dump documentation Chaitanya Kulkarni
2019-07-08 18:49   ` Chaitanya Kulkarni

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).