gfs2: Even more gfs2_find_jhead fixes
authorAndreas Gruenbacher <agruenba@redhat.com>
Tue, 26 May 2020 18:11:51 +0000 (20:11 +0200)
committerAndreas Gruenbacher <agruenba@redhat.com>
Fri, 29 May 2020 15:00:24 +0000 (17:00 +0200)
Fix several issues in the previous gfs2_find_jhead fix:
* When updating @blocks_submitted, @block refers to the first block block not
  submitted yet, not the last block submitted, so fix an off-by-one error.
* We want to ensure that @blocks_submitted is far enough ahead of @blocks_read
  to guarantee that there is in-flight I/O.  Otherwise, we'll eventually end up
  waiting for pages that haven't been submitted, yet.
* It's much easier to compare the number of blocks added with the number of
  blocks submitted to limit the maximum bio size.
* Even with bio chaining, we can keep adding blocks until we reach the maximum
  bio size, as long as we stop at a page boundary.  This simplifies the logic.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Bob Peterson <rpeterso@redhat.com>
fs/gfs2/lops.c

index 48b54ec..cb2a11b 100644 (file)
@@ -509,12 +509,12 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
        unsigned int bsize = sdp->sd_sb.sb_bsize, off;
        unsigned int bsize_shift = sdp->sd_sb.sb_bsize_shift;
        unsigned int shift = PAGE_SHIFT - bsize_shift;
-       unsigned int max_bio_size = 2 * 1024 * 1024;
+       unsigned int max_blocks = 2 * 1024 * 1024 >> bsize_shift;
        struct gfs2_journal_extent *je;
        int sz, ret = 0;
        struct bio *bio = NULL;
        struct page *page = NULL;
-       bool bio_chained = false, done = false;
+       bool done = false;
        errseq_t since;
 
        memset(head, 0, sizeof(*head));
@@ -537,10 +537,7 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
                                off = 0;
                        }
 
-                       if (!bio || (bio_chained && !off) ||
-                           bio->bi_iter.bi_size >= max_bio_size) {
-                               /* start new bio */
-                       } else {
+                       if (bio && (off || block < blocks_submitted + max_blocks)) {
                                sector_t sector = dblock << sdp->sd_fsb2bb_shift;
 
                                if (bio_end_sector(bio) == sector) {
@@ -553,19 +550,17 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
                                                (PAGE_SIZE - off) >> bsize_shift;
 
                                        bio = gfs2_chain_bio(bio, blocks);
-                                       bio_chained = true;
                                        goto add_block_to_new_bio;
                                }
                        }
 
                        if (bio) {
-                               blocks_submitted = block + 1;
+                               blocks_submitted = block;
                                submit_bio(bio);
                        }
 
                        bio = gfs2_log_alloc_bio(sdp, dblock, gfs2_end_log_read);
                        bio->bi_opf = REQ_OP_READ;
-                       bio_chained = false;
 add_block_to_new_bio:
                        sz = bio_add_page(bio, page, bsize, off);
                        BUG_ON(sz != bsize);
@@ -573,7 +568,7 @@ block_added:
                        off += bsize;
                        if (off == PAGE_SIZE)
                                page = NULL;
-                       if (blocks_submitted < 2 * max_bio_size >> bsize_shift) {
+                       if (blocks_submitted <= blocks_read + max_blocks) {
                                /* Keep at least one bio in flight */
                                continue;
                        }