btrfs: send: in case of IO error log inode

[PATCH] btrfs: send: in case of IO error log inode

Posted by Dāvis Mosāns 4 years, 4 months ago

Currently if we get IO error while doing send then we abort without
logging information about which file caused issue.
So log inode to help with debugging.
---
 fs/btrfs/send.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index d8ccb62aa7d2..945d9c01d902 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5000,6 +5000,7 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
 			if (!PageUptodate(page)) {
 				unlock_page(page);
 				put_page(page);
+				btrfs_err(fs_info, "received IO error for inode=%llu", sctx->cur_ino);
 				ret = -EIO;
 				break;
 			}
-- 
2.35.1

Re: [PATCH] btrfs: send: in case of IO error log inode

Posted by David Sterba 4 years, 4 months ago

On Wed, Feb 02, 2022 at 10:14:37PM +0200, Dāvis Mosāns wrote:
> Currently if we get IO error while doing send then we abort without
> logging information about which file caused issue.
> So log inode to help with debugging.

The Signed-off-by tag is missing.

> ---
>  fs/btrfs/send.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index d8ccb62aa7d2..945d9c01d902 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5000,6 +5000,7 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
>  			if (!PageUptodate(page)) {
>  				unlock_page(page);
>  				put_page(page);
> +				btrfs_err(fs_info, "received IO error for inode=%llu", sctx->cur_ino);

A message here makes sense. I'd make it more explicit that it's for send
(the word "received" is kind of confusing), the inode number is not
unique identifier so the root id should be also printed, it's available
from the sctx->send_root and maybe also the file offset (taken from the
associated page by page_offset()).

Re: [PATCH] btrfs: send: in case of IO error log inode

Posted by Dāvis Mosāns 4 years, 4 months ago

piektd., 2022. g. 4. febr., plkst. 18:29 — lietotājs David Sterba
(<dsterba@suse.cz>) rakstīja:
>
> On Wed, Feb 02, 2022 at 10:14:37PM +0200, Dāvis Mosāns wrote:
> > Currently if we get IO error while doing send then we abort without
> > logging information about which file caused issue.
> > So log inode to help with debugging.
>
> The Signed-off-by tag is missing.
>

Yeah I forgot.

> > ---
> >  fs/btrfs/send.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index d8ccb62aa7d2..945d9c01d902 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -5000,6 +5000,7 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
> >                       if (!PageUptodate(page)) {
> >                               unlock_page(page);
> >                               put_page(page);
> > +                             btrfs_err(fs_info, "received IO error for inode=%llu", sctx->cur_ino);
>
> A message here makes sense. I'd make it more explicit that it's for send
> (the word "received" is kind of confusing), the inode number is not
> unique identifier so the root id should be also printed, it's available
> from the sctx->send_root and maybe also the file offset (taken from the
> associated page by page_offset()).

Thanks! Will submit v2

[PATCH v2] btrfs: send: in case of IO error log it

Posted by Dāvis Mosāns 4 years, 4 months ago

Currently if we get IO error while doing send then we abort without
logging information about which file caused issue.
So log it to help with debugging.

Signed-off-by: Dāvis Mosāns <davispuh@gmail.com>
---
 fs/btrfs/send.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index d8ccb62aa7d2..a1fd449a5ecc 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5000,6 +5000,8 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
 			if (!PageUptodate(page)) {
 				unlock_page(page);
 				put_page(page);
+				btrfs_err(fs_info, "send: IO error at offset=%llu for inode=%llu root=%llu",
+					page_offset(page), sctx->cur_ino, sctx->send_root->root_key.objectid);
 				ret = -EIO;
 				break;
 			}
-- 
2.35.1

Re: [PATCH v2] btrfs: send: in case of IO error log it

Posted by Dāvis Mosāns 4 years, 4 months ago

> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index d8ccb62aa7d2..a1fd449a5ecc 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5000,6 +5000,8 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
>                         if (!PageUptodate(page)) {
>                                 unlock_page(page);
>                                 put_page(page);
> +                               btrfs_err(fs_info, "send: IO error at offset=%llu for inode=%llu root=%llu",
> +                                       page_offset(page), sctx->cur_ino, sctx->send_root->root_key.objectid);

Thought more about this and I'm guessing using page after put_page is
probably wrong but it did work fine. Submitted v3.

[PATCH v3] btrfs: send: in case of IO error log it

Posted by Dāvis Mosāns 4 years, 4 months ago

Currently if we get IO error while doing send then we abort without
logging information about which file caused issue.
So log it to help with debugging.

Signed-off-by: Dāvis Mosāns <davispuh@gmail.com>
---
 fs/btrfs/send.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index d8ccb62aa7d2..b1f75fde4a19 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4999,6 +4999,8 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
 			lock_page(page);
 			if (!PageUptodate(page)) {
 				unlock_page(page);
+				btrfs_err(fs_info, "send: IO error at offset=%llu for inode=%llu root=%llu",
+					page_offset(page), sctx->cur_ino, sctx->send_root->root_key.objectid);
 				put_page(page);
 				ret = -EIO;
 				break;
-- 
2.35.1

Re: [PATCH v3] btrfs: send: in case of IO error log it

Posted by David Sterba 4 years, 4 months ago

On Sat, Feb 05, 2022 at 08:48:23PM +0200, Dāvis Mosāns wrote:
> Currently if we get IO error while doing send then we abort without
> logging information about which file caused issue.
> So log it to help with debugging.
> 
> Signed-off-by: Dāvis Mosāns <davispuh@gmail.com>

Added to misc-next, thanks.

> ---
>  fs/btrfs/send.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index d8ccb62aa7d2..b1f75fde4a19 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -4999,6 +4999,8 @@ static int put_file_data(struct send_ctx *sctx, u64 offset, u32 len)
>  			lock_page(page);
>  			if (!PageUptodate(page)) {
>  				unlock_page(page);
> +				btrfs_err(fs_info, "send: IO error at offset=%llu for inode=%llu root=%llu",
> +					page_offset(page), sctx->cur_ino, sctx->send_root->root_key.objectid);
>  				put_page(page);

Good point in v2, using page must be before put_page. I've slightly
reformatted the message so the lines fit to the limit.