HAMMER VFS - Handle critical I/O errors without panicing

Physically pull the SATA cable while pounding the disk via a HAMMER
mounted filesystem and fix the error paths that are not handled properly.
Make umount -f work properly.

Reminder: Currently if HAMMER hits a critical media error it drops down
into read-only mode and can only be read or unmounted after that.

* Do not try to test the CRC for bread data if the bread fails.

* Clean up the reference count on hammer_buffer structures related to
  dangling modified and ioerror bits.

* Remove a debugging kprintf() that was lousing up the console output.

diff --git a/sys/vfs/hammer/hammer.h b/sys/vfs/hammer/hammer.h
index 579db22..43cd17f 100644 (file)
--- a/sys/vfs/hammer/hammer.h
+++ b/sys/vfs/hammer/hammer.h
@@ -1210,6 +1210,7 @@ void hammer_io_done_interlock(hammer_io_t io);
 void hammer_io_clear_modify(struct hammer_io *io, int inval);
 void hammer_io_clear_modlist(struct hammer_io *io);
 void hammer_io_flush_sync(hammer_mount_t hmp);
+void hammer_io_clear_error(struct hammer_io *io);
 
 void hammer_modify_volume(hammer_transaction_t trans, hammer_volume_t volume,
                        void *base, int len);

diff --git a/sys/vfs/hammer/hammer_btree.c b/sys/vfs/hammer/hammer_btree.c
index c79b835..6ee1e1a 100644 (file)
--- a/sys/vfs/hammer/hammer_btree.c
+++ b/sys/vfs/hammer/hammer_btree.c
@@ -726,7 +726,8 @@ hammer_btree_extract(hammer_cursor_t cursor, int flags)
        KKASSERT(data_len >= 0 && data_len <= HAMMER_XBUFSIZE);
        cursor->data = hammer_bread_ext(hmp, data_off, data_len,
                                        &error, &cursor->data_buffer);
-       if (hammer_crc_test_leaf(cursor->data, &elm->leaf) == 0) {
+       if (error == 0 &&
+           hammer_crc_test_leaf(cursor->data, &elm->leaf) == 0) {
                kprintf("CRC DATA @ %016llx/%d FAILED\n",
                        (long long)elm->leaf.data_offset, elm->leaf.data_len);
                if (hammer_debug_critical)

diff --git a/sys/vfs/hammer/hammer_io.c b/sys/vfs/hammer/hammer_io.c
index 66adcfb..f7dd622 100644 (file)
--- a/sys/vfs/hammer/hammer_io.c
+++ b/sys/vfs/hammer/hammer_io.c
@@ -159,6 +159,21 @@ hammer_io_wait_all(hammer_mount_t hmp, const char *ident)
        crit_exit();
 }
 
+/*
+ * Clear a flagged error condition on a I/O buffer.  The caller must hold
+ * its own ref on the buffer.
+ */
+void
+hammer_io_clear_error(struct hammer_io *io)
+{
+       if (io->ioerror) {
+               io->ioerror = 0;
+               hammer_unref(&io->lock);
+               KKASSERT(io->lock.refs > 0);
+       }
+}
+
+
 #define HAMMER_MAXRA   4
 
 /*

diff --git a/sys/vfs/hammer/hammer_ondisk.c b/sys/vfs/hammer/hammer_ondisk.c
index e6f948c..0888a21 100644 (file)
--- a/sys/vfs/hammer/hammer_ondisk.c
+++ b/sys/vfs/hammer/hammer_ondisk.c
@@ -506,7 +506,7 @@ hammer_mountcheck_volumes(struct hammer_mount *hmp)
  *                             BUFFERS                                 *
  ************************************************************************
  *
- * Manage buffers.  Currently all blockmap-backed zones are direct-mapped
+ * Manage buffers.  Currently most blockmap-backed zones are direct-mapped
  * to zone-2 buffer offsets, without a translation stage.  However, the
  * hammer_buffer structure is indexed by its zoneX_offset, not its
  * zone2_offset.
@@ -639,10 +639,10 @@ again:
         * Insert the buffer into the RB tree and handle late collisions.
         */
        if (RB_INSERT(hammer_buf_rb_tree, &hmp->rb_bufs_root, buffer)) {
-               hammer_unref(&buffer->io.lock); /* safety */
-               --hammer_count_buffers;
                hammer_rel_volume(volume, 0);
                buffer->io.volume = NULL;       /* safety */
+               hammer_unref(&buffer->io.lock); /* safety */
+               --hammer_count_buffers;
                kfree(buffer, hmp->m_misc);
                goto again;
        }
@@ -657,11 +657,13 @@ found:
                if (*errorp) {
                        hammer_rel_buffer(buffer, 1);
                        buffer = NULL;
+               } else {
+                       hammer_io_advance(&buffer->io);
                }
        } else {
                *errorp = 0;
+               hammer_io_advance(&buffer->io);
        }
-       hammer_io_advance(&buffer->io);
        return(buffer);
 }
 

diff --git a/sys/vfs/hammer/hammer_recover.c b/sys/vfs/hammer/hammer_recover.c
index cebc6e0..f03a45f 100644 (file)
--- a/sys/vfs/hammer/hammer_recover.c
+++ b/sys/vfs/hammer/hammer_recover.c
@@ -867,7 +867,9 @@ hammer_recover_debug_dump(int w, char *buf, int bytes)
 /*
  * Flush recovered buffers from recovery operations.  The call to this
  * routine may be delayed if a read-only mount was made and then later
- * upgraded to read-write.
+ * upgraded to read-write.  This routine is also called when unmounting
+ * a read-only mount to clean out recovered (dirty) buffers which we
+ * couldn't flush (because the mount is read-only).
  *
  * The volume header is always written last.  The UNDO FIFO will be forced
  * to zero-length by setting next_offset to first_offset.  This leaves the
@@ -927,6 +929,10 @@ hammer_recover_flush_buffers(hammer_mount_t hmp, hammer_volume_t root_volume,
  * all volume headers (including the root volume) will be discarded.
  * Otherwise data is the root_volume and we flush all volume headers
  * EXCEPT the root_volume.
+ *
+ * Clear any I/O error or modified condition when discarding buffers to
+ * clean up the reference count, otherwise the buffer may have extra refs
+ * on it.
  */
 static
 int
@@ -936,15 +942,24 @@ hammer_recover_flush_volume_callback(hammer_volume_t volume, void *data)
 
        if (volume->io.recovered && volume != root_volume) {
                volume->io.recovered = 0;
-               if (root_volume != NULL)
+               if (root_volume != NULL) {
                        hammer_io_flush(&volume->io, 0);
-               else
+               } else {
+                       hammer_io_clear_error(&volume->io);
                        hammer_io_clear_modify(&volume->io, 1);
+               }
                hammer_rel_volume(volume, 0);
        }
        return(0);
 }
 
+/*
+ * Flush or discard recovered I/O buffers.
+ *
+ * Clear any I/O error or modified condition when discarding buffers to
+ * clean up the reference count, otherwise the buffer may have extra refs
+ * on it.
+ */
 static
 int
 hammer_recover_flush_buffer_callback(hammer_buffer_t buffer, void *data)
@@ -954,15 +969,22 @@ hammer_recover_flush_buffer_callback(hammer_buffer_t buffer, void *data)
        if (buffer->io.recovered) {
                buffer->io.recovered = 0;
                buffer->io.reclaim = 1;
-               if (final < 0)
+               if (final < 0) {
+                       hammer_io_clear_error(&buffer->io);
                        hammer_io_clear_modify(&buffer->io, 1);
-               else
+               } else {
                        hammer_io_flush(&buffer->io, 0);
+               }
                hammer_rel_buffer(buffer, 0);
        } else {
-               KKASSERT(buffer->io.lock.refs == 0);
-               ++hammer_count_refedbufs;
+               if (buffer->io.lock.refs == 0)
+                       ++hammer_count_refedbufs;
                hammer_ref(&buffer->io.lock);
+               if (final < 0) {
+                       hammer_io_clear_error(&buffer->io);
+                       hammer_io_clear_modify(&buffer->io, 1);
+               }
+               KKASSERT(buffer->io.lock.refs == 1);
                buffer->io.reclaim = 1;
                hammer_rel_buffer(buffer, 1);
        }

diff --git a/sys/vfs/hammer/hammer_vnops.c b/sys/vfs/hammer/hammer_vnops.c
index 3e4625b..31f0228 100644 (file)
--- a/sys/vfs/hammer/hammer_vnops.c
+++ b/sys/vfs/hammer/hammer_vnops.c
@@ -362,7 +362,6 @@ hammer_vop_read(struct vop_read_args *ap)
                        error = bread(ap->a_vp, base_offset, blksize, &bp);
                }
                if (error) {
-                       kprintf("error %d\n", error);
                        brelse(bp);
                        break;
                }