Re: [Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head
- Original Message -
> From: "Andreas Gruenbacher"
> To: "Abhi Das"
> Cc: "cluster-devel"
> 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 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
> > ---
> > 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
> > #include
> >
> > +#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(>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_flags)) {
> > + mempool_free(page, gfs2_page_pool);
> > + continue;
> > + }
> > +
> > + if (!__get_log_header(sdp, ptr, 0, )) {
> > + if (lh.lh_sequence > jd->jd_jhead.lh_sequence)
> > + jd->jd_jhead = lh;
> > + else
> > + goto found;
> > + }
> > +
> > + if (last) {
> > + found:
> > + clear_bit(JDF_JHEAD, >jd_flags);
> > + wake_up_bit(>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(>sd_log_in_flight);
> > + if (op != REQ_OP_READ) {
> > + struct gfs2_sbd *sdp = bio->bi_private;
> > + atomic_inc(>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,
Re: [Cluster-devel] [GFS2 PATCH 4/4] gfs2: read journal in large chunks to locate the head
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 > --- (snip) > + for (i=0; i + 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
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 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
> ---
> 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
> #include
>
> +#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(>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_flags)) {
> + mempool_free(page, gfs2_page_pool);
> + continue;
> + }
> +
> + if (!__get_log_header(sdp, ptr, 0, )) {
> + if (lh.lh_sequence > jd->jd_jhead.lh_sequence)
> + jd->jd_jhead = lh;
> + else
> + goto found;
> + }
> +
> + if (last) {
> + found:
> + clear_bit(JDF_JHEAD, >jd_flags);
> + wake_up_bit(>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(>sd_log_in_flight);
> + if (op != REQ_OP_READ) {
> + struct gfs2_sbd *sdp = bio->bi_private;
> + atomic_inc(>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 ==
Re: [Cluster-devel] [GFS2 PATCH 0/4] Speed up journal head lookup
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(-)
