* [PATCH] btrfs: print error if primary super block write fails
@ 2018-01-26 22:47 Howard McLauchlan
2018-01-27 4:49 ` Qu Wenruo
2018-02-02 16:20 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Howard McLauchlan @ 2018-01-26 22:47 UTC (permalink / raw
To: linux-btrfs; +Cc: kernel-team, Omar Sandoval, Josef Bacik, Howard McLauchlan
Presently, failing a primary super block write but succeeding in at
least one super block write in general will appear to users as if
nothing important went wrong. However, upon unmounting and re-mounting,
the file system will be in a rolled back state. This was discovered
with a BCC program that uses bpf_override_return() to fail super block
writes.
This patch outputs an error clarifying that the primary super block
write has failed, so users can expect potentially erroneous behaviour.
It also forces wait_dev_supers() to return an error to its caller if
the primary super block write fails.
Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
---
fs/btrfs/disk-io.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 5da18ebc9222..8f96e1e4c613 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3293,11 +3293,13 @@ static int write_dev_supers(struct btrfs_device *device,
* Return number of errors when buffer head is not found or not marked up to
* date.
*/
-static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
+static int wait_dev_supers(struct btrfs_fs_info *fs_info,
+ struct btrfs_device *device, int max_mirrors)
{
struct buffer_head *bh;
int i;
int errors = 0;
+ bool primary_failed = false;
u64 bytenr;
if (max_mirrors == 0)
@@ -3314,11 +3316,14 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
BTRFS_SUPER_INFO_SIZE);
if (!bh) {
errors++;
+ primary_failed = (i == 0) || primary_failed;
continue;
}
wait_on_buffer(bh);
- if (!buffer_uptodate(bh))
+ if (!buffer_uptodate(bh)) {
errors++;
+ primary_failed = (i == 0) || primary_failed;
+ }
/* drop our reference */
brelse(bh);
@@ -3327,6 +3332,12 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
brelse(bh);
}
+ /* log error, force error return */
+ if (primary_failed) {
+ btrfs_err(fs_info, "error encountered writing primary super block");
+ return -1;
+ }
+
return errors < i ? 0 : -1;
}
@@ -3557,7 +3568,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
if (!dev->in_fs_metadata || !dev->writeable)
continue;
- ret = wait_dev_supers(dev, max_mirrors);
+ ret = wait_dev_supers(fs_info, dev, max_mirrors);
if (ret)
total_errors++;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: print error if primary super block write fails
2018-01-26 22:47 [PATCH] btrfs: print error if primary super block write fails Howard McLauchlan
@ 2018-01-27 4:49 ` Qu Wenruo
2018-02-02 16:20 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2018-01-27 4:49 UTC (permalink / raw
To: Howard McLauchlan, linux-btrfs; +Cc: kernel-team, Omar Sandoval, Josef Bacik
[-- Attachment #1.1: Type: text/plain, Size: 2837 bytes --]
On 2018年01月27日 06:47, Howard McLauchlan wrote:
> Presently, failing a primary super block write but succeeding in at
> least one super block write in general will appear to users as if
> nothing important went wrong. However, upon unmounting and re-mounting,
> the file system will be in a rolled back state. This was discovered
> with a BCC program that uses bpf_override_return() to fail super block
> writes.
>
> This patch outputs an error clarifying that the primary super block
> write has failed, so users can expect potentially erroneous behaviour.
> It also forces wait_dev_supers() to return an error to its caller if
> the primary super block write fails.
>
> Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
> ---
> fs/btrfs/disk-io.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 5da18ebc9222..8f96e1e4c613 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3293,11 +3293,13 @@ static int write_dev_supers(struct btrfs_device *device,
> * Return number of errors when buffer head is not found or not marked up to
> * date.
> */
> -static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> +static int wait_dev_supers(struct btrfs_fs_info *fs_info,
fs_info can be fetch from device->fs_info.
> + struct btrfs_device *device, int max_mirrors)
> {
> struct buffer_head *bh;
> int i;
> int errors = 0;
> + bool primary_failed = false;
> u64 bytenr;
>
> if (max_mirrors == 0)
> @@ -3314,11 +3316,14 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> BTRFS_SUPER_INFO_SIZE);
> if (!bh) {
> errors++;
> + primary_failed = (i == 0) || primary_failed;
> continue;
> }
> wait_on_buffer(bh);
> - if (!buffer_uptodate(bh))
> + if (!buffer_uptodate(bh)) {
> errors++;
> + primary_failed = (i == 0) || primary_failed;
> + }
>
> /* drop our reference */
> brelse(bh);
> @@ -3327,6 +3332,12 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> brelse(bh);
> }
>
> + /* log error, force error return */
> + if (primary_failed) {
> + btrfs_err(fs_info, "error encountered writing primary super block");
It would be better if device id can also be outputted.
Otherwise it looks good.
Thanks,
Qu
> + return -1;
> + }
> +
> return errors < i ? 0 : -1;
> }
>
> @@ -3557,7 +3568,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
> if (!dev->in_fs_metadata || !dev->writeable)
> continue;
>
> - ret = wait_dev_supers(dev, max_mirrors);
> + ret = wait_dev_supers(fs_info, dev, max_mirrors);
> if (ret)
> total_errors++;
> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: print error if primary super block write fails
2018-01-26 22:47 [PATCH] btrfs: print error if primary super block write fails Howard McLauchlan
2018-01-27 4:49 ` Qu Wenruo
@ 2018-02-02 16:20 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2018-02-02 16:20 UTC (permalink / raw
To: Howard McLauchlan; +Cc: linux-btrfs, kernel-team, Omar Sandoval, Josef Bacik
On Fri, Jan 26, 2018 at 02:47:09PM -0800, Howard McLauchlan wrote:
> Presently, failing a primary super block write but succeeding in at
> least one super block write in general will appear to users as if
> nothing important went wrong. However, upon unmounting and re-mounting,
> the file system will be in a rolled back state. This was discovered
> with a BCC program that uses bpf_override_return() to fail super block
> writes.
>
> This patch outputs an error clarifying that the primary super block
> write has failed, so users can expect potentially erroneous behaviour.
> It also forces wait_dev_supers() to return an error to its caller if
> the primary super block write fails.
>
> Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
> ---
> fs/btrfs/disk-io.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 5da18ebc9222..8f96e1e4c613 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3293,11 +3293,13 @@ static int write_dev_supers(struct btrfs_device *device,
> * Return number of errors when buffer head is not found or not marked up to
> * date.
> */
> -static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> +static int wait_dev_supers(struct btrfs_fs_info *fs_info,
> + struct btrfs_device *device, int max_mirrors)
> {
> struct buffer_head *bh;
> int i;
> int errors = 0;
> + bool primary_failed = false;
> u64 bytenr;
>
> if (max_mirrors == 0)
> @@ -3314,11 +3316,14 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> BTRFS_SUPER_INFO_SIZE);
> if (!bh) {
> errors++;
> + primary_failed = (i == 0) || primary_failed;
if (i == 0)
primary_failed = true;
looks more readable to me.
> continue;
> }
> wait_on_buffer(bh);
> - if (!buffer_uptodate(bh))
> + if (!buffer_uptodate(bh)) {
> errors++;
> + primary_failed = (i == 0) || primary_failed;
> + }
>
> /* drop our reference */
> brelse(bh);
> @@ -3327,6 +3332,12 @@ static int wait_dev_supers(struct btrfs_device *device, int max_mirrors)
> brelse(bh);
> }
>
> + /* log error, force error return */
> + if (primary_failed) {
> + btrfs_err(fs_info, "error encountered writing primary super block");
As Qu pointed out, the device id is desired here. The errno is probably
-EIO in all cases, so does not need to be part of the message.
> + return -1;
> + }
> +
> return errors < i ? 0 : -1;
> }
>
> @@ -3557,7 +3568,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
> if (!dev->in_fs_metadata || !dev->writeable)
> continue;
>
> - ret = wait_dev_supers(dev, max_mirrors);
> + ret = wait_dev_supers(fs_info, dev, max_mirrors);
> if (ret)
> total_errors++;
> }
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-02 16:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-26 22:47 [PATCH] btrfs: print error if primary super block write fails Howard McLauchlan
2018-01-27 4:49 ` Qu Wenruo
2018-02-02 16:20 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.