[Cluster-devel] [GFS2 PATCH] GFS2: Reduce code redundancy writing log headers
Bob Peterson
rpeterso at redhat.com
Tue Dec 19 18:55:59 UTC 2017
Hi,
Before this patch, there was a lot of code redundancy between functions
log_write_header (which uses bio) and clean_journal (which uses
buffer_head). This patch reduces the redundancy to simplify the code
and make log header writing more consistent. We want more consistency
and reduced redundancy because we plan to add a bunch of new fields
to improve performance (by eliminating the local statfs and quota files)
improve metadata integrity (by adding new crcs and such) and for better
debugging (by adding new fields to track when and where metadata was
pushed through the journals.) We don't want to duplicate setting these
new fields, nor allow for human error in the process.
This reduction in code redundancy is accomplished by introducing a new
helper function, gfs2_write_log_header which uses bio rather than bh.
That accomplishes three things: (1) It simplifies recovery function
clean_journal() to use the new helper function and iomap rather than
redundancy and block_map (and eventually we can maybe remove block_map).
(2) It adds new error checking of the bio request, if told to wait for
IO completion. (3) It reduces the dependency on buffer_heads.
Signed-off-by: Bob Peterson <rpeterso at redhat.com>
---
fs/gfs2/log.c | 50 ++++++++++++++++++++++++++++++++++---------------
fs/gfs2/log.h | 3 +++
fs/gfs2/lops.c | 24 +++++++++++++++++++-----
fs/gfs2/lops.h | 3 ++-
fs/gfs2/recovery.c | 55 ++++++++++++++----------------------------------------
fs/gfs2/util.c | 19 +++++++++++++++++++
fs/gfs2/util.h | 5 +++++
7 files changed, 97 insertions(+), 62 deletions(-)
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index f72c44231406..20cb9e3e4a95 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -648,48 +648,68 @@ void gfs2_write_revokes(struct gfs2_sbd *sdp)
}
/**
- * log_write_header - Get and initialize a journal header buffer
+ * write_log_header - Get and initialize a journal header buffer
* @sdp: The GFS2 superblock
+ * @seq: sequence number
+ * @tail: tail of the log
+ * @blkno: block number for the write
+ * @op_flags: flags to pass to the bio
+ * @wait: Should we wait for IO completion
*
* Returns: the initialized log buffer descriptor
*/
-static void log_write_header(struct gfs2_sbd *sdp, u32 flags)
+int gfs2_write_log_header(struct gfs2_sbd *sdp, u64 seq, u32 tail,
+ u32 blkno, u32 flags, int op_flags, bool wait)
{
struct gfs2_log_header *lh;
- unsigned int tail;
u32 hash;
- int op_flags = REQ_PREFLUSH | REQ_FUA | REQ_META | REQ_SYNC;
struct page *page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
- enum gfs2_freeze_state state = atomic_read(&sdp->sd_freeze_state);
+
lh = page_address(page);
clear_page(lh);
- gfs2_assert_withdraw(sdp, (state != SFS_FROZEN));
-
- tail = current_tail(sdp);
-
lh->lh_header.mh_magic = cpu_to_be32(GFS2_MAGIC);
lh->lh_header.mh_type = cpu_to_be32(GFS2_METATYPE_LH);
lh->lh_header.__pad0 = cpu_to_be64(0);
lh->lh_header.mh_format = cpu_to_be32(GFS2_FORMAT_LH);
lh->lh_header.mh_jid = cpu_to_be32(sdp->sd_jdesc->jd_jid);
- lh->lh_sequence = cpu_to_be64(sdp->sd_log_sequence++);
+ lh->lh_sequence = cpu_to_be64(seq);
lh->lh_flags = cpu_to_be32(flags);
lh->lh_tail = cpu_to_be32(tail);
- lh->lh_blkno = cpu_to_be32(sdp->sd_log_flush_head);
+ lh->lh_blkno = cpu_to_be32(blkno);
hash = gfs2_disk_hash(page_address(page), sizeof(struct gfs2_log_header));
lh->lh_hash = cpu_to_be32(hash);
+ gfs2_log_write_page(sdp, page);
+ return gfs2_log_flush_bio(sdp, REQ_OP_WRITE, op_flags, wait);
+}
+
+/**
+ * log_write_header - Get and initialize a journal header buffer
+ * @sdp: The GFS2 superblock
+ *
+ * Returns: the initialized log buffer descriptor
+ */
+
+static void log_write_header(struct gfs2_sbd *sdp, u32 flags)
+{
+ unsigned int tail;
+ int op_flags = REQ_PREFLUSH | REQ_FUA | REQ_META | REQ_SYNC;
+ enum gfs2_freeze_state state = atomic_read(&sdp->sd_freeze_state);
+
+ gfs2_assert_withdraw(sdp, (state != SFS_FROZEN));
+ tail = current_tail(sdp);
+
if (test_bit(SDF_NOBARRIERS, &sdp->sd_flags)) {
gfs2_ordered_wait(sdp);
log_flush_wait(sdp);
op_flags = REQ_SYNC | REQ_META | REQ_PRIO;
}
-
sdp->sd_log_idle = (tail == sdp->sd_log_flush_head);
- gfs2_log_write_page(sdp, page);
- gfs2_log_flush_bio(sdp, REQ_OP_WRITE, op_flags);
+ gfs2_write_log_header(sdp, sdp->sd_log_sequence++, tail,
+ sdp->sd_log_flush_head, flags, op_flags, false);
+
log_flush_wait(sdp);
if (sdp->sd_log_tail != tail)
@@ -739,7 +759,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl,
gfs2_ordered_write(sdp);
lops_before_commit(sdp, tr);
- gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0);
+ gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0, false);
if (sdp->sd_log_head != sdp->sd_log_flush_head) {
log_flush_wait(sdp);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index 9499a6049212..7727b5de7146 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -71,6 +71,9 @@ enum gfs2_flush_type {
SHUTDOWN_FLUSH,
FREEZE_FLUSH
};
+extern int gfs2_write_log_header(struct gfs2_sbd *sdp, u64 seq, u32 tail,
+ u32 blkno, u32 flags, int op_flags,
+ bool wait);
extern void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl,
enum gfs2_flush_type type);
extern void gfs2_log_commit(struct gfs2_sbd *sdp, struct gfs2_trans *trans);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index c8ff7b7954f0..2fd9157b17e0 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -231,19 +231,33 @@ static void gfs2_end_log_write(struct bio *bio)
* @sdp: The superblock
* @op: REQ_OP
* @op_flags: req_flag_bits
+ * @wait: Should we wait for the IO to complete
+ *
+ * Returns: error
*
* Submit any pending part-built or full bio to the block device. If
- * there is no pending bio, then this is a no-op.
+ * there is no pending bio, then this is a no-op. If told to wait, it returns
+ * -EIO if the bio failed.
*/
-void gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags)
+int gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags, bool wait)
{
+ int error = 0;
+
if (sdp->sd_log_bio) {
atomic_inc(&sdp->sd_log_in_flight);
bio_set_op_attrs(sdp->sd_log_bio, op, op_flags);
- submit_bio(sdp->sd_log_bio);
+ if (wait) {
+ if (submit_bio_wait(sdp->sd_log_bio)) {
+ gfs2_io_error_bio(sdp);
+ error = -EIO;
+ }
+ } else {
+ submit_bio(sdp->sd_log_bio);
+ }
sdp->sd_log_bio = NULL;
}
+ return error;
}
/**
@@ -300,7 +314,7 @@ static struct bio *gfs2_log_get_bio(struct gfs2_sbd *sdp, u64 blkno)
nblk >>= sdp->sd_fsb2bb_shift;
if (blkno == nblk)
return bio;
- gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0);
+ gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0, false);
}
return gfs2_log_alloc_bio(sdp, blkno);
@@ -329,7 +343,7 @@ static void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
bio = gfs2_log_get_bio(sdp, blkno);
ret = bio_add_page(bio, page, size, offset);
if (ret == 0) {
- gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0);
+ gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0, false);
bio = gfs2_log_alloc_bio(sdp, blkno);
ret = bio_add_page(bio, page, size, offset);
WARN_ON(ret == 0);
diff --git a/fs/gfs2/lops.h b/fs/gfs2/lops.h
index e529f536c117..8b35aff2fe9e 100644
--- a/fs/gfs2/lops.h
+++ b/fs/gfs2/lops.h
@@ -27,7 +27,8 @@ extern const struct gfs2_log_operations gfs2_databuf_lops;
extern const struct gfs2_log_operations *gfs2_log_ops[];
extern void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page);
-extern void gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags);
+extern int gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags,
+ bool wait);
extern void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh);
static inline unsigned int buf_limit(struct gfs2_sbd *sdp)
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 9395a3db1a60..d496863106fa 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -20,6 +20,7 @@
#include "bmap.h"
#include "glock.h"
#include "glops.h"
+#include "log.h"
#include "lops.h"
#include "meta_io.h"
#include "recovery.h"
@@ -370,9 +371,7 @@ static int foreach_descriptor(struct gfs2_jdesc *jd, unsigned int start,
/**
* clean_journal - mark a dirty journal as being clean
- * @sdp: the filesystem
* @jd: the journal
- * @gl: the journal's glock
* @head: the head journal to start from
*
* Returns: errno
@@ -380,52 +379,26 @@ static int foreach_descriptor(struct gfs2_jdesc *jd, unsigned int start,
static int clean_journal(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
{
- struct gfs2_inode *ip = GFS2_I(jd->jd_inode);
struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+ struct inode *inode = jd->jd_inode;
+ struct iomap iomap;
unsigned int lblock;
- struct gfs2_log_header *lh;
- u32 hash;
- struct buffer_head *bh;
- int error;
- struct buffer_head bh_map = { .b_state = 0, .b_blocknr = 0 };
+ int ret;
lblock = head->lh_blkno;
gfs2_replay_incr_blk(jd, &lblock);
- bh_map.b_size = 1 << ip->i_inode.i_blkbits;
- error = gfs2_block_map(&ip->i_inode, lblock, &bh_map, 0);
- if (error)
- return error;
- if (!bh_map.b_blocknr) {
- gfs2_consist_inode(ip);
- return -EIO;
- }
- bh = sb_getblk(sdp->sd_vfs, bh_map.b_blocknr);
- lock_buffer(bh);
- memset(bh->b_data, 0, bh->b_size);
- set_buffer_uptodate(bh);
- clear_buffer_dirty(bh);
- unlock_buffer(bh);
-
- lh = (struct gfs2_log_header *)bh->b_data;
- memset(lh, 0, sizeof(struct gfs2_log_header));
- lh->lh_header.mh_magic = cpu_to_be32(GFS2_MAGIC);
- lh->lh_header.mh_type = cpu_to_be32(GFS2_METATYPE_LH);
- lh->lh_header.__pad0 = cpu_to_be64(0);
- lh->lh_header.mh_format = cpu_to_be32(GFS2_FORMAT_LH);
- lh->lh_header.mh_jid = cpu_to_be32(sdp->sd_jdesc->jd_jid);
- lh->lh_sequence = cpu_to_be64(head->lh_sequence + 1);
- lh->lh_flags = cpu_to_be32(GFS2_LOG_HEAD_UNMOUNT);
- lh->lh_blkno = cpu_to_be32(lblock);
- hash = gfs2_disk_hash((const char *)lh, sizeof(struct gfs2_log_header));
- lh->lh_hash = cpu_to_be32(hash);
-
- set_buffer_dirty(bh);
- if (sync_dirty_buffer(bh))
- gfs2_io_error_bh(sdp, bh);
- brelse(bh);
+ ret = gfs2_iomap_begin(inode, (loff_t)lblock << inode->i_blkbits,
+ 1 << inode->i_blkbits, IOMAP_REPORT, &iomap);
+ if (ret)
+ return ret;
+ if (iomap.type != IOMAP_MAPPED)
+ return -EIO;
- return error;
+ return gfs2_write_log_header(sdp, head->lh_sequence + 1, 0,
+ iomap.addr, GFS2_LOG_HEAD_UNMOUNT,
+ REQ_PREFLUSH | REQ_FUA | REQ_META |
+ REQ_SYNC, true);
}
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 763d659db91b..3cc436c5a74a 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/spinlock.h>
+#include <linux/bio.h>
#include <linux/completion.h>
#include <linux/buffer_head.h>
#include <linux/crc32.h>
@@ -264,3 +265,21 @@ int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
return rv;
}
+/**
+ * gfs2_io_error_bio_i - Flag a bio I/O error and withdraw
+ * Returns: -1 if this call withdrew the machine,
+ * 0 if it was already withdrawn
+ */
+
+int gfs2_io_error_bio_i(struct gfs2_sbd *sdp, const char *function,
+ char *file, unsigned int line)
+{
+ int rv;
+ rv = gfs2_lm_withdraw(sdp,
+ "fatal: I/O error\n"
+ " block = %llu\n"
+ " function = %s, file = %s, line = %u\n",
+ (unsigned long long)sdp->sd_log_bio->bi_iter.bi_sector << sdp->sd_fsb2bb_shift,
+ function, file, line);
+ return rv;
+}
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index 3926f95a6eb7..e863f642e2c4 100644
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -142,6 +142,11 @@ int gfs2_io_error_bh_i(struct gfs2_sbd *sdp, struct buffer_head *bh,
#define gfs2_io_error_bh(sdp, bh) \
gfs2_io_error_bh_i((sdp), (bh), __func__, __FILE__, __LINE__);
+int gfs2_io_error_bio_i(struct gfs2_sbd *sdp, const char *function,
+ char *file, unsigned int line);
+
+#define gfs2_io_error_bio(sdp) \
+gfs2_io_error_bio_i((sdp), __func__, __FILE__, __LINE__);
extern struct kmem_cache *gfs2_glock_cachep;
extern struct kmem_cache *gfs2_glock_aspace_cachep;
More information about the Cluster-devel
mailing list