GFS2: Reduce code redundancy writing log headers
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 GFS2: Reduce code redundancy writing log headers
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 <[email protected]>
--- 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(-)
fs/gfs2/log.c +35 -15
@@ -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);
fs/gfs2/log.h +3 -0
@@ -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);
fs/gfs2/lops.c +19 -5
@@ -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 );
fs/gfs2/lops.h +2 -1
@@ -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)
fs/gfs2/recovery.c +14 -41
81 lines not on this page · raw message
fs/gfs2/util.c +19 -0
@@ -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; +}
fs/gfs2/util.h +5 -0
@@ -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;
PATCH 1/1 GFS2: Reduce code redundancy writing log headers
----- Original Message -----
| 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 <[email protected]> | ---
Hi,
Self-NACK. I found a bug during testing, so I'll need to rework this.
Bob Peterson