[Cluster-devel] [GFS2 PATCH 0/4] Speed up journal head lookup

[Cluster-devel] [GFS2 PATCH 0/4] Speed up journal head lookup

[Abhi Das]

This is the upstream version of the rhel7 patchset I'd
posted earlier for review.

It is slightly different in parts owing to some bits
already being present and the hash/crc computation code
being different due to the updated log header structure.

Cheers!
--Abhi


*** BLURB HERE ***

Abhi Das (4):
  gfs2: add timing info to map_journal_extents
  gfs2: changes to gfs2_log_XXX_bio
  gfs2: add a helper function to get_log_header that can be used
    elsewhere
  gfs2: read journal in large chunks to locate the head

 fs/gfs2/bmap.c       |   8 ++-
 fs/gfs2/incore.h     |   8 ++-
 fs/gfs2/log.c        |   4 +-
 fs/gfs2/lops.c       | 142 ++++++++++++++++++++++++++++++++++++-------
 fs/gfs2/lops.h       |   3 +-
 fs/gfs2/ops_fstype.c |   1 +
 fs/gfs2/recovery.c   | 168 +++++++++++++--------------------------------------
 fs/gfs2/recovery.h   |   2 +
 8 files changed, 184 insertions(+), 152 deletions(-)

-- 
2.4.11

[Cluster-devel] [GFS2 PATCH 1/4] gfs2: add timing info to map_journal_extents

[Abhi Das]

Tells you how many milliseconds map_journal_extents takes.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/bmap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 03128ed..dddb5a4 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -14,6 +14,7 @@
 #include <linux/gfs2_ondisk.h>
 #include <linux/crc32.h>
 #include <linux/iomap.h>
+#include <linux/ktime.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -2248,7 +2249,9 @@ int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd)
 	unsigned int shift = sdp->sd_sb.sb_bsize_shift;
 	u64 size;
 	int rc;
+	ktime_t start, end;
 
+	start = ktime_get();
 	lblock_stop = i_size_read(jd->jd_inode) >> shift;
 	size = (lblock_stop - lblock) << shift;
 	jd->nr_extents = 0;
@@ -2268,8 +2271,9 @@ int gfs2_map_journal_extents(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd)
 		lblock += (bh.b_size >> ip->i_inode.i_blkbits);
 	} while(size > 0);
 
-	fs_info(sdp, "journal %d mapped with %u extents\n", jd->jd_jid,
-		jd->nr_extents);
+	end = ktime_get();
+	fs_info(sdp, "journal %d mapped with %u extents in %lldms\n", jd->jd_jid,
+		jd->nr_extents, ktime_ms_delta(end, start));
 	return 0;
 
 fail:
-- 
2.4.11

[Cluster-devel] [GFS2 PATCH 2/4] gfs2: changes to gfs2_log_XXX_bio

[Abhi Das]

Change gfs2_log_flush_bio to accept a pointer to the struct bio*
to be flushed. Change gfs2_log_alloc_bio and gfs2_log_get_bio to
take a struct gfs2_jdesc* instead of gfs2_sbd.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/log.c  |  4 ++--
 fs/gfs2/lops.c | 34 +++++++++++++++++++---------------
 fs/gfs2/lops.h |  2 +-
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index ee20ea42..b80fb30 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -731,7 +731,7 @@ void gfs2_write_log_header(struct gfs2_sbd *sdp, struct gfs2_jdesc *jd,
 	lh->lh_crc = cpu_to_be32(crc);
 
 	gfs2_log_write(sdp, page, sb->s_blocksize, 0, addr);
-	gfs2_log_flush_bio(sdp, REQ_OP_WRITE, op_flags);
+	gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, op_flags);
 	log_flush_wait(sdp);
 }
 
@@ -808,7 +808,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
 
 	gfs2_ordered_write(sdp);
 	lops_before_commit(sdp, tr);
-	gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0);
+	gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
 
 	if (sdp->sd_log_head != sdp->sd_log_flush_head) {
 		log_flush_wait(sdp);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index f2567f9..4cc19af 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -229,7 +229,7 @@ static void gfs2_end_log_write(struct bio *bio)
 
 /**
  * gfs2_log_flush_bio - Submit any pending log bio
- * @sdp: The superblock
+ * @biop: Address of the bio pointer
  * @op: REQ_OP
  * @op_flags: req_flag_bits
  *
@@ -237,19 +237,21 @@ static void gfs2_end_log_write(struct bio *bio)
  * there is no pending bio, then this is a no-op.
  */
 
-void gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags)
+void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
 {
-	if (sdp->sd_log_bio) {
+	struct bio *bio = *biop;
+	if (bio) {
+		struct gfs2_sbd *sdp = bio->bi_private;
 		atomic_inc(&sdp->sd_log_in_flight);
-		bio_set_op_attrs(sdp->sd_log_bio, op, op_flags);
-		submit_bio(sdp->sd_log_bio);
-		sdp->sd_log_bio = NULL;
+		bio_set_op_attrs(bio, op, op_flags);
+		submit_bio(bio);
+		*biop = NULL;
 	}
 }
 
 /**
  * gfs2_log_alloc_bio - Allocate a new bio for log writing
- * @sdp: The superblock
+ * @jd: The journal descriptor
  * @blkno: The next device block number we want to write to
  *
  * This should never be called when there is a cached bio in the
@@ -260,8 +262,9 @@ void gfs2_log_flush_bio(struct gfs2_sbd *sdp, int op, int op_flags)
  * Returns: Newly allocated bio
  */
 
-static struct bio *gfs2_log_alloc_bio(struct gfs2_sbd *sdp, u64 blkno)
+static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
 {
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
 	struct super_block *sb = sdp->sd_vfs;
 	struct bio *bio;
 
@@ -280,7 +283,7 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_sbd *sdp, u64 blkno)
 
 /**
  * gfs2_log_get_bio - Get cached log bio, or allocate a new one
- * @sdp: The superblock
+ * @jd: The journal descriptor
  * @blkno: The device block number we want to write to
  *
  * If there is a cached bio, then if the next block number is sequential
@@ -291,8 +294,9 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_sbd *sdp, u64 blkno)
  * Returns: The bio to use for log writes
  */
 
-static struct bio *gfs2_log_get_bio(struct gfs2_sbd *sdp, u64 blkno)
+static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
 {
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
 	struct bio *bio = sdp->sd_log_bio;
 	u64 nblk;
 
@@ -301,10 +305,10 @@ 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->sd_log_bio, REQ_OP_WRITE, 0);
 	}
 
-	return gfs2_log_alloc_bio(sdp, blkno);
+	return gfs2_log_alloc_bio(sdp->sd_jdesc, blkno);
 }
 
 /**
@@ -326,11 +330,11 @@ void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
 	struct bio *bio;
 	int ret;
 
-	bio = gfs2_log_get_bio(sdp, blkno);
+	bio = gfs2_log_get_bio(sdp->sd_jdesc, blkno);
 	ret = bio_add_page(bio, page, size, offset);
 	if (ret == 0) {
-		gfs2_log_flush_bio(sdp, REQ_OP_WRITE, 0);
-		bio = gfs2_log_alloc_bio(sdp, blkno);
+		gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
+		bio = gfs2_log_alloc_bio(sdp->sd_jdesc, 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 e494939..d709d99 100644
--- a/fs/gfs2/lops.h
+++ b/fs/gfs2/lops.h
@@ -30,7 +30,7 @@ extern u64 gfs2_log_bmap(struct gfs2_sbd *sdp);
 extern void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
 			   unsigned size, unsigned offset, u64 blkno);
 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 void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags);
 extern void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh);
 
 static inline unsigned int buf_limit(struct gfs2_sbd *sdp)
-- 
2.4.11

[Cluster-devel] [GFS2 PATCH 3/4] gfs2: add a helper function to get_log_header that can be used elsewhere

[Abhi Das]

Move and re-order the error checks and hash/crc computations into another
function __get_log_header() so it can be used in scenarios where buffer_heads
are not being used for the log header.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/recovery.c | 53 ++++++++++++++++++++++++++++++++---------------------
 fs/gfs2/recovery.h |  2 ++
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 0f501f9..1b95294 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -120,6 +120,35 @@ void gfs2_revoke_clean(struct gfs2_jdesc *jd)
 	}
 }
 
+int __get_log_header(struct gfs2_sbd *sdp, const struct gfs2_log_header *lh,
+		     unsigned int blkno, struct gfs2_log_header_host *head)
+{
+	u32 hash, crc;
+
+	if (lh->lh_header.mh_magic != cpu_to_be32(GFS2_MAGIC) ||
+	    lh->lh_header.mh_type != cpu_to_be32(GFS2_METATYPE_LH) ||
+	    (blkno && be32_to_cpu(lh->lh_blkno) != blkno))
+		return 1;
+
+	hash = crc32(~0, lh, LH_V1_SIZE - 4);
+	hash = ~crc32_le_shift(hash, 4); /* assume lh_hash is zero */
+
+	if (be32_to_cpu(lh->lh_hash) != hash)
+		return 1;
+
+	crc = crc32c(~0, (void *)lh + LH_V1_SIZE + 4,
+		     sdp->sd_sb.sb_bsize - LH_V1_SIZE - 4);
+
+	if ((lh->lh_crc != 0 && be32_to_cpu(lh->lh_crc) != crc))
+		return 1;
+
+	head->lh_sequence = be64_to_cpu(lh->lh_sequence);
+	head->lh_flags = be32_to_cpu(lh->lh_flags);
+	head->lh_tail = be32_to_cpu(lh->lh_tail);
+	head->lh_blkno = be32_to_cpu(lh->lh_blkno);
+
+	return 0;
+}
 /**
  * get_log_header - read the log header for a given segment
  * @jd: the journal
@@ -137,36 +166,18 @@ void gfs2_revoke_clean(struct gfs2_jdesc *jd)
 static int get_log_header(struct gfs2_jdesc *jd, unsigned int blk,
 			  struct gfs2_log_header_host *head)
 {
-	struct gfs2_log_header *lh;
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
 	struct buffer_head *bh;
-	u32 hash, crc;
 	int error;
 
 	error = gfs2_replay_read_block(jd, blk, &bh);
 	if (error)
 		return error;
-	lh = (void *)bh->b_data;
-
-	hash = crc32(~0, lh, LH_V1_SIZE - 4);
-	hash = ~crc32_le_shift(hash, 4);  /* assume lh_hash is zero */
-
-	crc = crc32c(~0, (void *)lh + LH_V1_SIZE + 4,
-		     bh->b_size - LH_V1_SIZE - 4);
-
-	error = lh->lh_header.mh_magic != cpu_to_be32(GFS2_MAGIC) ||
-		lh->lh_header.mh_type != cpu_to_be32(GFS2_METATYPE_LH) ||
-		be32_to_cpu(lh->lh_blkno) != blk ||
-		be32_to_cpu(lh->lh_hash) != hash ||
-		(lh->lh_crc != 0 && be32_to_cpu(lh->lh_crc) != crc);
 
+	error = __get_log_header(sdp, (const struct gfs2_log_header *)bh->b_data,
+				 blk, head);
 	brelse(bh);
 
-	if (!error) {
-		head->lh_sequence = be64_to_cpu(lh->lh_sequence);
-		head->lh_flags = be32_to_cpu(lh->lh_flags);
-		head->lh_tail = be32_to_cpu(lh->lh_tail);
-		head->lh_blkno = be32_to_cpu(lh->lh_blkno);
-	}
 	return error;
 }
 
diff --git a/fs/gfs2/recovery.h b/fs/gfs2/recovery.h
index 11fdfab..943a67c 100644
--- a/fs/gfs2/recovery.h
+++ b/fs/gfs2/recovery.h
@@ -31,6 +31,8 @@ extern int gfs2_find_jhead(struct gfs2_jdesc *jd,
 		    struct gfs2_log_header_host *head);
 extern int gfs2_recover_journal(struct gfs2_jdesc *gfs2_jd, bool wait);
 extern void gfs2_recover_func(struct work_struct *work);
+extern int __get_log_header(struct gfs2_sbd *sdp, const struct gfs2_log_header *lh,
+			    unsigned int blkno, struct gfs2_log_header_host *head);
 
 #endif /* __RECOVERY_DOT_H__ */
 
-- 
2.4.11

[Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head

[Abhi Das]

Use bio(s) to read in the journal sequentially in large chunks and
locate the head of the journal.
This is faster in most cases when compared to the existing bisect
method which operates one block at a time.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/incore.h     |   8 +++-
 fs/gfs2/lops.c       | 122 +++++++++++++++++++++++++++++++++++++++++++++------
 fs/gfs2/lops.h       |   1 +
 fs/gfs2/ops_fstype.c |   1 +
 fs/gfs2/recovery.c   | 115 +++++-------------------------------------------
 5 files changed, 129 insertions(+), 118 deletions(-)

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index b96d39c..b24c105 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -529,6 +529,11 @@ struct gfs2_journal_extent {
 	u64 blocks;
 };
 
+enum {
+	JDF_RECOVERY = 1,
+	JDF_JHEAD    = 2,
+};
+
 struct gfs2_jdesc {
 	struct list_head jd_list;
 	struct list_head extent_list;
@@ -536,12 +541,13 @@ struct gfs2_jdesc {
 	struct work_struct jd_work;
 	struct inode *jd_inode;
 	unsigned long jd_flags;
-#define JDF_RECOVERY 1
 	unsigned int jd_jid;
 	unsigned int jd_blocks;
 	int jd_recover_error;
 	/* Replay stuff */
 
+	struct gfs2_log_header_host jd_jhead;
+	struct bio *jd_rd_bio; /* bio used for reading this journal */
 	unsigned int jd_found_blocks;
 	unsigned int jd_found_revokes;
 	unsigned int jd_replayed_blocks;
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 4cc19af..21979b2 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/list_sort.h>
 
+#include "bmap.h"
 #include "dir.h"
 #include "gfs2.h"
 #include "incore.h"
@@ -227,6 +228,50 @@ static void gfs2_end_log_write(struct bio *bio)
 		wake_up(&sdp->sd_log_flush_wait);
 }
 
+static void gfs2_end_log_read(struct bio *bio)
+{
+	struct gfs2_jdesc *jd = bio->bi_private;
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+	struct page *page;
+	struct bio_vec *bvec;
+	int i, last;
+
+	if (bio->bi_status) {
+		fs_err(sdp, "Error %d reading from journal, jid=%u\n",
+		       bio->bi_status, jd->jd_jid);
+	}
+
+	bio_for_each_segment_all(bvec, bio, i) {
+		struct gfs2_log_header_host uninitialized_var(lh);
+		void *ptr;
+
+		page = bvec->bv_page;
+		ptr = page_address(page);
+		last = page_private(page);
+
+		if (!test_bit(JDF_JHEAD, &jd->jd_flags)) {
+			mempool_free(page, gfs2_page_pool);
+			continue;
+		}
+
+		if (!__get_log_header(sdp, ptr, 0, &lh)) {
+			if (lh.lh_sequence > jd->jd_jhead.lh_sequence)
+				jd->jd_jhead = lh;
+			else
+				goto found;
+		}
+
+		if (last) {
+		found:
+			clear_bit(JDF_JHEAD, &jd->jd_flags);
+			wake_up_bit(&jd->jd_flags, JDF_JHEAD);
+		}
+		mempool_free(page, gfs2_page_pool);
+	}
+
+	bio_put(bio);
+}
+
 /**
  * gfs2_log_flush_bio - Submit any pending log bio
  * @biop: Address of the bio pointer
@@ -241,8 +286,10 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
 {
 	struct bio *bio = *biop;
 	if (bio) {
-		struct gfs2_sbd *sdp = bio->bi_private;
-		atomic_inc(&sdp->sd_log_in_flight);
+		if (op != REQ_OP_READ) {
+			struct gfs2_sbd *sdp = bio->bi_private;
+			atomic_inc(&sdp->sd_log_in_flight);
+		}
 		bio_set_op_attrs(bio, op, op_flags);
 		submit_bio(bio);
 		*biop = NULL;
@@ -253,6 +300,7 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
  * gfs2_log_alloc_bio - Allocate a new bio for log writing
  * @jd: The journal descriptor
  * @blkno: The next device block number we want to write to
+ * @op: REQ_OP
  *
  * This should never be called when there is a cached bio in the
  * super block. When it returns, there will be a cached bio in the
@@ -262,21 +310,24 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
  * Returns: Newly allocated bio
  */
 
-static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
+static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno, int op)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
 	struct super_block *sb = sdp->sd_vfs;
 	struct bio *bio;
 
-	BUG_ON(sdp->sd_log_bio);
+	BUG_ON((op == REQ_OP_READ ? jd->jd_rd_bio : sdp->sd_log_bio));
 
 	bio = bio_alloc(GFP_NOIO, BIO_MAX_PAGES);
 	bio->bi_iter.bi_sector = blkno * (sb->s_blocksize >> 9);
 	bio_set_dev(bio, sb->s_bdev);
-	bio->bi_end_io = gfs2_end_log_write;
-	bio->bi_private = sdp;
+	bio->bi_end_io = op == REQ_OP_READ ? gfs2_end_log_read : gfs2_end_log_write;
+	bio->bi_private = op == REQ_OP_READ ? (void*)jd : (void*)sdp;
 
-	sdp->sd_log_bio = bio;
+	if (op == REQ_OP_READ)
+		jd->jd_rd_bio = bio;
+	else
+		sdp->sd_log_bio = bio;
 
 	return bio;
 }
@@ -285,6 +336,7 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
  * gfs2_log_get_bio - Get cached log bio, or allocate a new one
  * @jd: The journal descriptor
  * @blkno: The device block number we want to write to
+ * @op: REQ_OP
  *
  * If there is a cached bio, then if the next block number is sequential
  * with the previous one, return it, otherwise flush the bio to the
@@ -294,10 +346,10 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
  * Returns: The bio to use for log writes
  */
 
-static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
+static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno, int op)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
-	struct bio *bio = sdp->sd_log_bio;
+	struct bio *bio = op == REQ_OP_READ ? jd->jd_rd_bio : sdp->sd_log_bio;
 	u64 nblk;
 
 	if (bio) {
@@ -305,10 +357,12 @@ static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
 		nblk >>= sdp->sd_fsb2bb_shift;
 		if (blkno == nblk)
 			return bio;
-		gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
+		gfs2_log_flush_bio(op == REQ_OP_READ ? &jd->jd_rd_bio
+				   : &sdp->sd_log_bio, REQ_OP_WRITE, 0);
 	}
 
-	return gfs2_log_alloc_bio(sdp->sd_jdesc, blkno);
+	return gfs2_log_alloc_bio(op == REQ_OP_READ ? jd : sdp->sd_jdesc,
+				  blkno, op);
 }
 
 /**
@@ -330,11 +384,11 @@ void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
 	struct bio *bio;
 	int ret;
 
-	bio = gfs2_log_get_bio(sdp->sd_jdesc, blkno);
+	bio = gfs2_log_get_bio(sdp->sd_jdesc, blkno, REQ_OP_WRITE);
 	ret = bio_add_page(bio, page, size, offset);
 	if (ret == 0) {
 		gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
-		bio = gfs2_log_alloc_bio(sdp->sd_jdesc, blkno);
+		bio = gfs2_log_alloc_bio(sdp->sd_jdesc, blkno, REQ_OP_WRITE);
 		ret = bio_add_page(bio, page, size, offset);
 		WARN_ON(ret == 0);
 	}
@@ -374,6 +428,48 @@ void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page)
 		       gfs2_log_bmap(sdp));
 }
 
+static void gfs2_log_read_extent(struct gfs2_jdesc *jd, u64 dblock,
+			  unsigned int blocks, int last)
+{
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+	struct super_block *sb = sdp->sd_vfs;
+	struct page *page;
+	int i, ret;
+	struct bio *bio;
+
+	for (i=0; i<blocks; i++) {
+		page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
+		/* flag the last page of the journal we plan to read in */
+		page_private(page) = (last && i == (blocks - 1));
+
+		bio = gfs2_log_get_bio(jd, dblock + i, REQ_OP_READ);
+		ret = bio_add_page(bio, page, sb->s_blocksize, 0);
+		if (ret == 0) {
+			gfs2_log_flush_bio(&jd->jd_rd_bio, REQ_OP_READ, 0);
+			bio = gfs2_log_alloc_bio(jd, dblock + i, REQ_OP_READ);
+			ret = bio_add_page(bio, page, sb->s_blocksize, 0);
+			WARN_ON(ret == 0);
+		}
+		bio->bi_private = jd;
+	}
+}
+
+void gfs2_log_read(struct gfs2_jdesc *jd)
+{
+	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
+	int last = 0;
+	struct gfs2_journal_extent *je;
+
+	if (list_empty(&jd->extent_list))
+		gfs2_map_journal_extents(sdp, jd);
+
+	list_for_each_entry(je, &jd->extent_list, list) {
+		last = list_is_last(&je->list, &jd->extent_list);
+		gfs2_log_read_extent(jd, je->dblock, je->blocks, last);
+		gfs2_log_flush_bio(&jd->jd_rd_bio, REQ_OP_READ, 0);
+	}
+}
+
 static struct page *gfs2_get_log_desc(struct gfs2_sbd *sdp, u32 ld_type,
 				      u32 ld_length, u32 ld_data1)
 {
diff --git a/fs/gfs2/lops.h b/fs/gfs2/lops.h
index d709d99..23392c5d 100644
--- a/fs/gfs2/lops.h
+++ b/fs/gfs2/lops.h
@@ -32,6 +32,7 @@ extern void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
 extern void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page);
 extern void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags);
 extern void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh);
+extern void gfs2_log_read(struct gfs2_jdesc *jd);
 
 static inline unsigned int buf_limit(struct gfs2_sbd *sdp)
 {
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c2469833b..dcc488b4 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -578,6 +578,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
 			kfree(jd);
 			break;
 		}
+		jd->jd_rd_bio = NULL;
 
 		spin_lock(&sdp->sd_jindex_spin);
 		jd->jd_jid = sdp->sd_journals++;
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 1b95294..e90abe6 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -182,85 +182,11 @@ static int get_log_header(struct gfs2_jdesc *jd, unsigned int blk,
 }
 
 /**
- * find_good_lh - find a good log header
- * @jd: the journal
- * @blk: the segment to start searching from
- * @lh: the log header to fill in
- * @forward: if true search forward in the log, else search backward
- *
- * Call get_log_header() to get a log header for a segment, but if the
- * segment is bad, either scan forward or backward until we find a good one.
- *
- * Returns: errno
- */
-
-static int find_good_lh(struct gfs2_jdesc *jd, unsigned int *blk,
-			struct gfs2_log_header_host *head)
-{
-	unsigned int orig_blk = *blk;
-	int error;
-
-	for (;;) {
-		error = get_log_header(jd, *blk, head);
-		if (error <= 0)
-			return error;
-
-		if (++*blk == jd->jd_blocks)
-			*blk = 0;
-
-		if (*blk == orig_blk) {
-			gfs2_consist_inode(GFS2_I(jd->jd_inode));
-			return -EIO;
-		}
-	}
-}
-
-/**
- * jhead_scan - make sure we've found the head of the log
- * @jd: the journal
- * @head: this is filled in with the log descriptor of the head
- *
- * At this point, seg and lh should be either the head of the log or just
- * before.  Scan forward until we find the head.
- *
- * Returns: errno
- */
-
-static int jhead_scan(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
-{
-	unsigned int blk = head->lh_blkno;
-	struct gfs2_log_header_host lh;
-	int error;
-
-	for (;;) {
-		if (++blk == jd->jd_blocks)
-			blk = 0;
-
-		error = get_log_header(jd, blk, &lh);
-		if (error < 0)
-			return error;
-		if (error == 1)
-			continue;
-
-		if (lh.lh_sequence == head->lh_sequence) {
-			gfs2_consist_inode(GFS2_I(jd->jd_inode));
-			return -EIO;
-		}
-		if (lh.lh_sequence < head->lh_sequence)
-			break;
-
-		*head = lh;
-	}
-
-	return 0;
-}
-
-/**
  * gfs2_find_jhead - find the head of a log
  * @jd: the journal
  * @head: the log descriptor for the head of the log is returned here
  *
- * Do a binary search of a journal and find the valid log entry with the
+ * Do a search of a journal and find the valid log entry with the
  * highest sequence number.  (i.e. the log head)
  *
  * Returns: errno
@@ -268,38 +194,19 @@ static int jhead_scan(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
 
 int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
 {
-	struct gfs2_log_header_host lh_1, lh_m;
-	u32 blk_1, blk_2, blk_m;
-	int error;
-
-	blk_1 = 0;
-	blk_2 = jd->jd_blocks - 1;
-
-	for (;;) {
-		blk_m = (blk_1 + blk_2) / 2;
-
-		error = find_good_lh(jd, &blk_1, &lh_1);
-		if (error)
-			return error;
-
-		error = find_good_lh(jd, &blk_m, &lh_m);
-		if (error)
-			return error;
-
-		if (blk_1 == blk_m || blk_m == blk_2)
-			break;
+	int error = 0;
 
-		if (lh_1.lh_sequence <= lh_m.lh_sequence)
-			blk_1 = blk_m;
-		else
-			blk_2 = blk_m;
-	}
+	memset(&jd->jd_jhead, 0, sizeof(struct gfs2_log_header_host));
+	set_bit(JDF_JHEAD, &jd->jd_flags);
+	gfs2_log_read(jd);
 
-	error = jhead_scan(jd, &lh_1);
-	if (error)
-		return error;
+	if (test_bit(JDF_JHEAD, &jd->jd_flags))
+		wait_on_bit(&jd->jd_flags, JDF_JHEAD, TASK_INTERRUPTIBLE);
 
-	*head = lh_1;
+	if (jd->jd_jhead.lh_sequence == 0)
+		error = 1;
+	else
+		*head = jd->jd_jhead;
 
 	return error;
 }
-- 
2.4.11

[Cluster-devel] [GFS2 PATCH 0/4] Speed up journal head lookup

[Steven Whitehouse]

Hi,


On 06/09/18 18:02, Abhi Das wrote:
> This is the upstream version of the rhel7 patchset I'd
> posted earlier for review.
>
> It is slightly different in parts owing to some bits
> already being present and the hash/crc computation code
> being different due to the updated log header structure.
>
> Cheers!
> --Abhi
Looks good. Thanks for sorting this out, it should be a good base on 
which to build future improvements. Lets make sure it gets lots of testing,

Steve.

>
> *** BLURB HERE ***
>
> Abhi Das (4):
>    gfs2: add timing info to map_journal_extents
>    gfs2: changes to gfs2_log_XXX_bio
>    gfs2: add a helper function to get_log_header that can be used
>      elsewhere
>    gfs2: read journal in large chunks to locate the head
>
>   fs/gfs2/bmap.c       |   8 ++-
>   fs/gfs2/incore.h     |   8 ++-
>   fs/gfs2/log.c        |   4 +-
>   fs/gfs2/lops.c       | 142 ++++++++++++++++++++++++++++++++++++-------
>   fs/gfs2/lops.h       |   3 +-
>   fs/gfs2/ops_fstype.c |   1 +
>   fs/gfs2/recovery.c   | 168 +++++++++++++--------------------------------------
>   fs/gfs2/recovery.h   |   2 +
>   8 files changed, 184 insertions(+), 152 deletions(-)
>

[Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head

[Andreas Gruenbacher]

Abhi,

On 6 September 2018 at 19:02, Abhi Das <adas@redhat.com> wrote:
> Use bio(s) to read in the journal sequentially in large chunks and
> locate the head of the journal.
> This is faster in most cases when compared to the existing bisect
> method which operates one block at a time.
>
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>  fs/gfs2/incore.h     |   8 +++-
>  fs/gfs2/lops.c       | 122 +++++++++++++++++++++++++++++++++++++++++++++------
>  fs/gfs2/lops.h       |   1 +
>  fs/gfs2/ops_fstype.c |   1 +
>  fs/gfs2/recovery.c   | 115 +++++-------------------------------------------
>  5 files changed, 129 insertions(+), 118 deletions(-)
>
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index b96d39c..b24c105 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -529,6 +529,11 @@ struct gfs2_journal_extent {
>         u64 blocks;
>  };
>
> +enum {
> +       JDF_RECOVERY = 1,
> +       JDF_JHEAD    = 2,
> +};
> +
>  struct gfs2_jdesc {
>         struct list_head jd_list;
>         struct list_head extent_list;
> @@ -536,12 +541,13 @@ struct gfs2_jdesc {
>         struct work_struct jd_work;
>         struct inode *jd_inode;
>         unsigned long jd_flags;
> -#define JDF_RECOVERY 1
>         unsigned int jd_jid;
>         unsigned int jd_blocks;
>         int jd_recover_error;
>         /* Replay stuff */
>
> +       struct gfs2_log_header_host jd_jhead;
> +       struct bio *jd_rd_bio; /* bio used for reading this journal */
>         unsigned int jd_found_blocks;
>         unsigned int jd_found_revokes;
>         unsigned int jd_replayed_blocks;
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 4cc19af..21979b2 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -18,6 +18,7 @@
>  #include <linux/fs.h>
>  #include <linux/list_sort.h>
>
> +#include "bmap.h"
>  #include "dir.h"
>  #include "gfs2.h"
>  #include "incore.h"
> @@ -227,6 +228,50 @@ static void gfs2_end_log_write(struct bio *bio)
>                 wake_up(&sdp->sd_log_flush_wait);
>  }
>
> +static void gfs2_end_log_read(struct bio *bio)
> +{
> +       struct gfs2_jdesc *jd = bio->bi_private;
> +       struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> +       struct page *page;
> +       struct bio_vec *bvec;
> +       int i, last;
> +
> +       if (bio->bi_status) {
> +               fs_err(sdp, "Error %d reading from journal, jid=%u\n",
> +                      bio->bi_status, jd->jd_jid);
> +       }
> +
> +       bio_for_each_segment_all(bvec, bio, i) {
> +               struct gfs2_log_header_host uninitialized_var(lh);
> +               void *ptr;
> +
> +               page = bvec->bv_page;
> +               ptr = page_address(page);
> +               last = page_private(page);
> +
> +               if (!test_bit(JDF_JHEAD, &jd->jd_flags)) {
> +                       mempool_free(page, gfs2_page_pool);
> +                       continue;
> +               }
> +
> +               if (!__get_log_header(sdp, ptr, 0, &lh)) {
> +                       if (lh.lh_sequence > jd->jd_jhead.lh_sequence)
> +                               jd->jd_jhead = lh;
> +                       else
> +                               goto found;
> +               }
> +
> +               if (last) {
> +               found:
> +                       clear_bit(JDF_JHEAD, &jd->jd_flags);
> +                       wake_up_bit(&jd->jd_flags, JDF_JHEAD);
> +               }
> +               mempool_free(page, gfs2_page_pool);
> +       }
> +
> +       bio_put(bio);
> +}
> +
>  /**
>   * gfs2_log_flush_bio - Submit any pending log bio
>   * @biop: Address of the bio pointer
> @@ -241,8 +286,10 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
>  {
>         struct bio *bio = *biop;
>         if (bio) {
> -               struct gfs2_sbd *sdp = bio->bi_private;
> -               atomic_inc(&sdp->sd_log_in_flight);
> +               if (op != REQ_OP_READ) {
> +                       struct gfs2_sbd *sdp = bio->bi_private;
> +                       atomic_inc(&sdp->sd_log_in_flight);
> +               }
>                 bio_set_op_attrs(bio, op, op_flags);
>                 submit_bio(bio);
>                 *biop = NULL;
> @@ -253,6 +300,7 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
>   * gfs2_log_alloc_bio - Allocate a new bio for log writing
>   * @jd: The journal descriptor
>   * @blkno: The next device block number we want to write to
> + * @op: REQ_OP
>   *
>   * This should never be called when there is a cached bio in the
>   * super block. When it returns, there will be a cached bio in the
> @@ -262,21 +310,24 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags)
>   * Returns: Newly allocated bio
>   */
>
> -static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
> +static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno, int op)
>  {
>         struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
>         struct super_block *sb = sdp->sd_vfs;
>         struct bio *bio;
>
> -       BUG_ON(sdp->sd_log_bio);
> +       BUG_ON((op == REQ_OP_READ ? jd->jd_rd_bio : sdp->sd_log_bio));
>
>         bio = bio_alloc(GFP_NOIO, BIO_MAX_PAGES);
>         bio->bi_iter.bi_sector = blkno * (sb->s_blocksize >> 9);
>         bio_set_dev(bio, sb->s_bdev);
> -       bio->bi_end_io = gfs2_end_log_write;
> -       bio->bi_private = sdp;
> +       bio->bi_end_io = op == REQ_OP_READ ? gfs2_end_log_read : gfs2_end_log_write;
> +       bio->bi_private = op == REQ_OP_READ ? (void*)jd : (void*)sdp;
>
> -       sdp->sd_log_bio = bio;
> +       if (op == REQ_OP_READ)
> +               jd->jd_rd_bio = bio;
> +       else
> +               sdp->sd_log_bio = bio;
>
>         return bio;
>  }
> @@ -285,6 +336,7 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
>   * gfs2_log_get_bio - Get cached log bio, or allocate a new one
>   * @jd: The journal descriptor
>   * @blkno: The device block number we want to write to
> + * @op: REQ_OP
>   *
>   * If there is a cached bio, then if the next block number is sequential
>   * with the previous one, return it, otherwise flush the bio to the
> @@ -294,10 +346,10 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
>   * Returns: The bio to use for log writes
>   */
>
> -static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
> +static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno, int op)
>  {
>         struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> -       struct bio *bio = sdp->sd_log_bio;
> +       struct bio *bio = op == REQ_OP_READ ? jd->jd_rd_bio : sdp->sd_log_bio;
>         u64 nblk;
>
>         if (bio) {
> @@ -305,10 +357,12 @@ static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
>                 nblk >>= sdp->sd_fsb2bb_shift;
>                 if (blkno == nblk)
>                         return bio;
> -               gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
> +               gfs2_log_flush_bio(op == REQ_OP_READ ? &jd->jd_rd_bio
> +                                  : &sdp->sd_log_bio, REQ_OP_WRITE, 0);

Shouldn't it be "op" here instead of "REQ_OP_WRITE"?

>         }
>
> -       return gfs2_log_alloc_bio(sdp->sd_jdesc, blkno);
> +       return gfs2_log_alloc_bio(op == REQ_OP_READ ? jd : sdp->sd_jdesc,
> +                                 blkno, op);
>  }
>
>  /**
> @@ -330,11 +384,11 @@ void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
>         struct bio *bio;
>         int ret;
>
> -       bio = gfs2_log_get_bio(sdp->sd_jdesc, blkno);
> +       bio = gfs2_log_get_bio(sdp->sd_jdesc, blkno, REQ_OP_WRITE);
>         ret = bio_add_page(bio, page, size, offset);
>         if (ret == 0) {
>                 gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
> -               bio = gfs2_log_alloc_bio(sdp->sd_jdesc, blkno);
> +               bio = gfs2_log_alloc_bio(sdp->sd_jdesc, blkno, REQ_OP_WRITE);
>                 ret = bio_add_page(bio, page, size, offset);
>                 WARN_ON(ret == 0);
>         }
> @@ -374,6 +428,48 @@ void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page)
>                        gfs2_log_bmap(sdp));
>  }
>
> +static void gfs2_log_read_extent(struct gfs2_jdesc *jd, u64 dblock,
> +                         unsigned int blocks, int last)
> +{
> +       struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> +       struct super_block *sb = sdp->sd_vfs;
> +       struct page *page;
> +       int i, ret;
> +       struct bio *bio;
> +
> +       for (i=0; i<blocks; i++) {
> +               page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
> +               /* flag the last page of the journal we plan to read in */
> +               page_private(page) = (last && i == (blocks - 1));
> +
> +               bio = gfs2_log_get_bio(jd, dblock + i, REQ_OP_READ);
> +               ret = bio_add_page(bio, page, sb->s_blocksize, 0);
> +               if (ret == 0) {
> +                       gfs2_log_flush_bio(&jd->jd_rd_bio, REQ_OP_READ, 0);
> +                       bio = gfs2_log_alloc_bio(jd, dblock + i, REQ_OP_READ);
> +                       ret = bio_add_page(bio, page, sb->s_blocksize, 0);
> +                       WARN_ON(ret == 0);
> +               }
> +               bio->bi_private = jd;
> +       }
> +}
> +
> +void gfs2_log_read(struct gfs2_jdesc *jd)
> +{
> +       struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> +       int last = 0;
> +       struct gfs2_journal_extent *je;
> +
> +       if (list_empty(&jd->extent_list))
> +               gfs2_map_journal_extents(sdp, jd);
> +
> +       list_for_each_entry(je, &jd->extent_list, list) {
> +               last = list_is_last(&je->list, &jd->extent_list);
> +               gfs2_log_read_extent(jd, je->dblock, je->blocks, last);
> +               gfs2_log_flush_bio(&jd->jd_rd_bio, REQ_OP_READ, 0);
> +       }
> +}
> +
>  static struct page *gfs2_get_log_desc(struct gfs2_sbd *sdp, u32 ld_type,
>                                       u32 ld_length, u32 ld_data1)
>  {
> diff --git a/fs/gfs2/lops.h b/fs/gfs2/lops.h
> index d709d99..23392c5d 100644
> --- a/fs/gfs2/lops.h
> +++ b/fs/gfs2/lops.h
> @@ -32,6 +32,7 @@ extern void gfs2_log_write(struct gfs2_sbd *sdp, struct page *page,
>  extern void gfs2_log_write_page(struct gfs2_sbd *sdp, struct page *page);
>  extern void gfs2_log_flush_bio(struct bio **biop, int op, int op_flags);
>  extern void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh);
> +extern void gfs2_log_read(struct gfs2_jdesc *jd);
>
>  static inline unsigned int buf_limit(struct gfs2_sbd *sdp)
>  {
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index c2469833b..dcc488b4 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -578,6 +578,7 @@ static int gfs2_jindex_hold(struct gfs2_sbd *sdp, struct gfs2_holder *ji_gh)
>                         kfree(jd);
>                         break;
>                 }
> +               jd->jd_rd_bio = NULL;
>
>                 spin_lock(&sdp->sd_jindex_spin);
>                 jd->jd_jid = sdp->sd_journals++;
> diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
> index 1b95294..e90abe6 100644
> --- a/fs/gfs2/recovery.c
> +++ b/fs/gfs2/recovery.c
> @@ -182,85 +182,11 @@ static int get_log_header(struct gfs2_jdesc *jd, unsigned int blk,
>  }
>
>  /**
> - * find_good_lh - find a good log header
> - * @jd: the journal
> - * @blk: the segment to start searching from
> - * @lh: the log header to fill in
> - * @forward: if true search forward in the log, else search backward
> - *
> - * Call get_log_header() to get a log header for a segment, but if the
> - * segment is bad, either scan forward or backward until we find a good one.
> - *
> - * Returns: errno
> - */
> -
> -static int find_good_lh(struct gfs2_jdesc *jd, unsigned int *blk,
> -                       struct gfs2_log_header_host *head)
> -{
> -       unsigned int orig_blk = *blk;
> -       int error;
> -
> -       for (;;) {
> -               error = get_log_header(jd, *blk, head);
> -               if (error <= 0)
> -                       return error;
> -
> -               if (++*blk == jd->jd_blocks)
> -                       *blk = 0;
> -
> -               if (*blk == orig_blk) {
> -                       gfs2_consist_inode(GFS2_I(jd->jd_inode));
> -                       return -EIO;
> -               }
> -       }
> -}
> -
> -/**
> - * jhead_scan - make sure we've found the head of the log
> - * @jd: the journal
> - * @head: this is filled in with the log descriptor of the head
> - *
> - * At this point, seg and lh should be either the head of the log or just
> - * before.  Scan forward until we find the head.
> - *
> - * Returns: errno
> - */
> -
> -static int jhead_scan(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
> -{
> -       unsigned int blk = head->lh_blkno;
> -       struct gfs2_log_header_host lh;
> -       int error;
> -
> -       for (;;) {
> -               if (++blk == jd->jd_blocks)
> -                       blk = 0;
> -
> -               error = get_log_header(jd, blk, &lh);
> -               if (error < 0)
> -                       return error;
> -               if (error == 1)
> -                       continue;
> -
> -               if (lh.lh_sequence == head->lh_sequence) {
> -                       gfs2_consist_inode(GFS2_I(jd->jd_inode));
> -                       return -EIO;
> -               }
> -               if (lh.lh_sequence < head->lh_sequence)
> -                       break;
> -
> -               *head = lh;
> -       }
> -
> -       return 0;
> -}
> -
> -/**
>   * gfs2_find_jhead - find the head of a log
>   * @jd: the journal
>   * @head: the log descriptor for the head of the log is returned here
>   *
> - * Do a binary search of a journal and find the valid log entry with the
> + * Do a search of a journal and find the valid log entry with the
>   * highest sequence number.  (i.e. the log head)
>   *
>   * Returns: errno
> @@ -268,38 +194,19 @@ static int jhead_scan(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
>
>  int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head)
>  {
> -       struct gfs2_log_header_host lh_1, lh_m;
> -       u32 blk_1, blk_2, blk_m;
> -       int error;
> -
> -       blk_1 = 0;
> -       blk_2 = jd->jd_blocks - 1;
> -
> -       for (;;) {
> -               blk_m = (blk_1 + blk_2) / 2;
> -
> -               error = find_good_lh(jd, &blk_1, &lh_1);
> -               if (error)
> -                       return error;
> -
> -               error = find_good_lh(jd, &blk_m, &lh_m);
> -               if (error)
> -                       return error;
> -
> -               if (blk_1 == blk_m || blk_m == blk_2)
> -                       break;
> +       int error = 0;
>
> -               if (lh_1.lh_sequence <= lh_m.lh_sequence)
> -                       blk_1 = blk_m;
> -               else
> -                       blk_2 = blk_m;
> -       }
> +       memset(&jd->jd_jhead, 0, sizeof(struct gfs2_log_header_host));
> +       set_bit(JDF_JHEAD, &jd->jd_flags);
> +       gfs2_log_read(jd);
>
> -       error = jhead_scan(jd, &lh_1);
> -       if (error)
> -               return error;
> +       if (test_bit(JDF_JHEAD, &jd->jd_flags))
> +               wait_on_bit(&jd->jd_flags, JDF_JHEAD, TASK_INTERRUPTIBLE);
>
> -       *head = lh_1;
> +       if (jd->jd_jhead.lh_sequence == 0)
> +               error = 1;
> +       else
> +               *head = jd->jd_jhead;
>
>         return error;
>  }
> --
> 2.4.11
>

Thanks,
Andreas

[Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head

[Bob Peterson]

Hi Abhi,

Overall, I like the patch set. What Andreas said still applies, plus:
----- Original Message -----
> Use bio(s) to read in the journal sequentially in large chunks and
> locate the head of the journal.
> This is faster in most cases when compared to the existing bisect
> method which operates one block at a time.
> 
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
(snip)
> +	for (i=0; i<blocks; i++) {

Just a nit: Can you put some spaces in here like this?
for (i = 0; i < blocks; i++) {

(snip)

> +	if (jd->jd_jhead.lh_sequence == 0)
> +		error = 1;

Since gfs2_find_jhead is called by remount, which is called from vfs,
I think we should probably use a return code more meaningful than 1.
I'm not sure what we ought to use in this case. Perhaps -EPERM or -EACCES ?

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head

[Abhijith Das]

----- Original Message -----
> From: "Andreas Gruenbacher" <agruenba@redhat.com>
> To: "Abhi Das" <adas@redhat.com>
> Cc: "cluster-devel" <cluster-devel@redhat.com>
> Sent: Friday, September 7, 2018 7:14:29 AM
> Subject: Re: [Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head
> 
> Abhi,
> 
> On 6 September 2018 at 19:02, Abhi Das <adas@redhat.com> wrote:
> > Use bio(s) to read in the journal sequentially in large chunks and
> > locate the head of the journal.
> > This is faster in most cases when compared to the existing bisect
> > method which operates one block at a time.
> >
> > Signed-off-by: Abhi Das <adas@redhat.com>
> > ---
> >  fs/gfs2/incore.h     |   8 +++-
> >  fs/gfs2/lops.c       | 122
> >  +++++++++++++++++++++++++++++++++++++++++++++------
> >  fs/gfs2/lops.h       |   1 +
> >  fs/gfs2/ops_fstype.c |   1 +
> >  fs/gfs2/recovery.c   | 115
> >  +++++-------------------------------------------
> >  5 files changed, 129 insertions(+), 118 deletions(-)
> >
> > diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> > index b96d39c..b24c105 100644
> > --- a/fs/gfs2/incore.h
> > +++ b/fs/gfs2/incore.h
> > @@ -529,6 +529,11 @@ struct gfs2_journal_extent {
> >         u64 blocks;
> >  };
> >
> > +enum {
> > +       JDF_RECOVERY = 1,
> > +       JDF_JHEAD    = 2,
> > +};
> > +
> >  struct gfs2_jdesc {
> >         struct list_head jd_list;
> >         struct list_head extent_list;
> > @@ -536,12 +541,13 @@ struct gfs2_jdesc {
> >         struct work_struct jd_work;
> >         struct inode *jd_inode;
> >         unsigned long jd_flags;
> > -#define JDF_RECOVERY 1
> >         unsigned int jd_jid;
> >         unsigned int jd_blocks;
> >         int jd_recover_error;
> >         /* Replay stuff */
> >
> > +       struct gfs2_log_header_host jd_jhead;
> > +       struct bio *jd_rd_bio; /* bio used for reading this journal */
> >         unsigned int jd_found_blocks;
> >         unsigned int jd_found_revokes;
> >         unsigned int jd_replayed_blocks;
> > diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> > index 4cc19af..21979b2 100644
> > --- a/fs/gfs2/lops.c
> > +++ b/fs/gfs2/lops.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/fs.h>
> >  #include <linux/list_sort.h>
> >
> > +#include "bmap.h"
> >  #include "dir.h"
> >  #include "gfs2.h"
> >  #include "incore.h"
> > @@ -227,6 +228,50 @@ static void gfs2_end_log_write(struct bio *bio)
> >                 wake_up(&sdp->sd_log_flush_wait);
> >  }
> >
> > +static void gfs2_end_log_read(struct bio *bio)
> > +{
> > +       struct gfs2_jdesc *jd = bio->bi_private;
> > +       struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> > +       struct page *page;
> > +       struct bio_vec *bvec;
> > +       int i, last;
> > +
> > +       if (bio->bi_status) {
> > +               fs_err(sdp, "Error %d reading from journal, jid=%u\n",
> > +                      bio->bi_status, jd->jd_jid);
> > +       }
> > +
> > +       bio_for_each_segment_all(bvec, bio, i) {
> > +               struct gfs2_log_header_host uninitialized_var(lh);
> > +               void *ptr;
> > +
> > +               page = bvec->bv_page;
> > +               ptr = page_address(page);
> > +               last = page_private(page);
> > +
> > +               if (!test_bit(JDF_JHEAD, &jd->jd_flags)) {
> > +                       mempool_free(page, gfs2_page_pool);
> > +                       continue;
> > +               }
> > +
> > +               if (!__get_log_header(sdp, ptr, 0, &lh)) {
> > +                       if (lh.lh_sequence > jd->jd_jhead.lh_sequence)
> > +                               jd->jd_jhead = lh;
> > +                       else
> > +                               goto found;
> > +               }
> > +
> > +               if (last) {
> > +               found:
> > +                       clear_bit(JDF_JHEAD, &jd->jd_flags);
> > +                       wake_up_bit(&jd->jd_flags, JDF_JHEAD);
> > +               }
> > +               mempool_free(page, gfs2_page_pool);
> > +       }
> > +
> > +       bio_put(bio);
> > +}
> > +
> >  /**
> >   * gfs2_log_flush_bio - Submit any pending log bio
> >   * @biop: Address of the bio pointer
> > @@ -241,8 +286,10 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int
> > op_flags)
> >  {
> >         struct bio *bio = *biop;
> >         if (bio) {
> > -               struct gfs2_sbd *sdp = bio->bi_private;
> > -               atomic_inc(&sdp->sd_log_in_flight);
> > +               if (op != REQ_OP_READ) {
> > +                       struct gfs2_sbd *sdp = bio->bi_private;
> > +                       atomic_inc(&sdp->sd_log_in_flight);
> > +               }
> >                 bio_set_op_attrs(bio, op, op_flags);
> >                 submit_bio(bio);
> >                 *biop = NULL;
> > @@ -253,6 +300,7 @@ void gfs2_log_flush_bio(struct bio **biop, int op, int
> > op_flags)
> >   * gfs2_log_alloc_bio - Allocate a new bio for log writing
> >   * @jd: The journal descriptor
> >   * @blkno: The next device block number we want to write to
> > + * @op: REQ_OP
> >   *
> >   * This should never be called when there is a cached bio in the
> >   * super block. When it returns, there will be a cached bio in the
> > @@ -262,21 +310,24 @@ void gfs2_log_flush_bio(struct bio **biop, int op,
> > int op_flags)
> >   * Returns: Newly allocated bio
> >   */
> >
> > -static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno)
> > +static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc *jd, u64 blkno,
> > int op)
> >  {
> >         struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> >         struct super_block *sb = sdp->sd_vfs;
> >         struct bio *bio;
> >
> > -       BUG_ON(sdp->sd_log_bio);
> > +       BUG_ON((op == REQ_OP_READ ? jd->jd_rd_bio : sdp->sd_log_bio));
> >
> >         bio = bio_alloc(GFP_NOIO, BIO_MAX_PAGES);
> >         bio->bi_iter.bi_sector = blkno * (sb->s_blocksize >> 9);
> >         bio_set_dev(bio, sb->s_bdev);
> > -       bio->bi_end_io = gfs2_end_log_write;
> > -       bio->bi_private = sdp;
> > +       bio->bi_end_io = op == REQ_OP_READ ? gfs2_end_log_read :
> > gfs2_end_log_write;
> > +       bio->bi_private = op == REQ_OP_READ ? (void*)jd : (void*)sdp;
> >
> > -       sdp->sd_log_bio = bio;
> > +       if (op == REQ_OP_READ)
> > +               jd->jd_rd_bio = bio;
> > +       else
> > +               sdp->sd_log_bio = bio;
> >
> >         return bio;
> >  }
> > @@ -285,6 +336,7 @@ static struct bio *gfs2_log_alloc_bio(struct gfs2_jdesc
> > *jd, u64 blkno)
> >   * gfs2_log_get_bio - Get cached log bio, or allocate a new one
> >   * @jd: The journal descriptor
> >   * @blkno: The device block number we want to write to
> > + * @op: REQ_OP
> >   *
> >   * If there is a cached bio, then if the next block number is sequential
> >   * with the previous one, return it, otherwise flush the bio to the
> > @@ -294,10 +346,10 @@ static struct bio *gfs2_log_alloc_bio(struct
> > gfs2_jdesc *jd, u64 blkno)
> >   * Returns: The bio to use for log writes
> >   */
> >
> > -static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno)
> > +static struct bio *gfs2_log_get_bio(struct gfs2_jdesc *jd, u64 blkno, int
> > op)
> >  {
> >         struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
> > -       struct bio *bio = sdp->sd_log_bio;
> > +       struct bio *bio = op == REQ_OP_READ ? jd->jd_rd_bio :
> > sdp->sd_log_bio;
> >         u64 nblk;
> >
> >         if (bio) {
> > @@ -305,10 +357,12 @@ static struct bio *gfs2_log_get_bio(struct gfs2_jdesc
> > *jd, u64 blkno)
> >                 nblk >>= sdp->sd_fsb2bb_shift;
> >                 if (blkno == nblk)
> >                         return bio;
> > -               gfs2_log_flush_bio(&sdp->sd_log_bio, REQ_OP_WRITE, 0);
> > +               gfs2_log_flush_bio(op == REQ_OP_READ ? &jd->jd_rd_bio
> > +                                  : &sdp->sd_log_bio, REQ_OP_WRITE, 0);
> 
> Shouldn't it be "op" here instead of "REQ_OP_WRITE"?
> 
Yep, you're right! Good catch. I'll repost this patch along with Bob's suggestions as well.

Cheers!
--Abhi