On 02/03/2018 03:09 AM, 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.

Right.! In case of non-datasync-IO, or where applications depend on the
transcation_commit() to confirm the IO.

David, Qu,

 In this context (single disk btrfs, the write to primary SB has
 been failing but read is fine), if a system is rebooted and continues
 the production the data loss is eminent!!!. btrfs_err() won't help to
 avoid the data loss nor we could blame the user for not taking any
 action on the btrfs_err().

 IMHO there are two choices to fix a. put the FS to read-only on the
 first write fail to the primary SB b. read backup SB at the time of
 the mount, as suggested [1]. Choice b is more appropriate, you may
 need to reconsider your opinion.

   [1]  https://patchwork.kernel.org/patch/10101725/


Thanks, Anand


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 <hmclauch...@fb.com>
---
V3: Rewrote boolean setting logic for clarity
V2: Added devid to output, removed unnecessary fs_info parameter

  fs/btrfs/disk-io.c | 15 ++++++++++++++-
  1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 5da18ebc9222..6ef32eb94669 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3298,6 +3298,7 @@ static int wait_dev_supers(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 +3315,16 @@ static int wait_dev_supers(struct btrfs_device *device, 
int max_mirrors)
                                      BTRFS_SUPER_INFO_SIZE);
                if (!bh) {
                        errors++;
+                       if (i == 0)
+                               primary_failed = true;
                        continue;
                }
                wait_on_buffer(bh);
-               if (!buffer_uptodate(bh))
+               if (!buffer_uptodate(bh)) {
                        errors++;
+                       if (i == 0)
+                               primary_failed = true;
+               }
/* drop our reference */
                brelse(bh);
@@ -3327,6 +3333,13 @@ static int wait_dev_supers(struct btrfs_device *device, 
int max_mirrors)
                brelse(bh);
        }
+ /* log error, force error return */
+       if (primary_failed) {
+               btrfs_err(device->fs_info, "error writing primary super block to 
device %llu",
+                         device->devid);
+               return -1;
+       }
+
        return errors < i ? 0 : -1;
  }
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to