xfs
[Top] [All Lists]

Re: [PATCH] xfs: deduplicate xlog_do_recovery_pass()

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfs: deduplicate xlog_do_recovery_pass()
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 21 Aug 2014 08:36:06 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <53F5651C.8030206@xxxxxxxxxx>
References: <53F5651C.8030206@xxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Wed, Aug 20, 2014 at 10:18:52PM -0500, Eric Sandeen wrote:
> In xlog_do_recovery_pass(), there are 2 distinct cases:
> non-wrapped and wrapped log recovery.
> 
> If we find a wrapped log, we recover around the end
> of the log, and then handle the rest of recovery
> exactly as in the non-wrapped case - using exactly the same
> (duplicated) code.
> 
> Rather than having the same code in both cases, we can
> get the wrapped portion out of the way first if needed,
> and then recover the non-wrapped portion of the log.
> 
> There should be no functional change here, just code
> reorganization & deduplication.
> 
> The patch looks a bit bigger than it really is; the last
> hunk is whitespace changes (un-indenting).
> 
> Tested with xfstests "check -g log" on a stock configuration.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---

Looks good to me:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

> 
>  xfs_log_recover.c |   81 
> ++++++++++++++++++------------------------------------
>  1 file changed, 27 insertions(+), 54 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 1fd5787..08d3569 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -4132,41 +4132,13 @@ xlog_do_recovery_pass(
>       }
>  
>       memset(rhash, 0, sizeof(rhash));
> -     if (tail_blk <= head_blk) {
> -             for (blk_no = tail_blk; blk_no < head_blk; ) {
> -                     error = xlog_bread(log, blk_no, hblks, hbp, &offset);
> -                     if (error)
> -                             goto bread_err2;
> -
> -                     rhead = (xlog_rec_header_t *)offset;
> -                     error = xlog_valid_rec_header(log, rhead, blk_no);
> -                     if (error)
> -                             goto bread_err2;
> -
> -                     /* blocks in data section */
> -                     bblks = (int)BTOBB(be32_to_cpu(rhead->h_len));
> -                     error = xlog_bread(log, blk_no + hblks, bblks, dbp,
> -                                        &offset);
> -                     if (error)
> -                             goto bread_err2;
> -
> -                     error = xlog_unpack_data(rhead, offset, log);
> -                     if (error)
> -                             goto bread_err2;
> -
> -                     error = xlog_recover_process_data(log,
> -                                             rhash, rhead, offset, pass);
> -                     if (error)
> -                             goto bread_err2;
> -                     blk_no += bblks + hblks;
> -             }
> -     } else {
> +     blk_no = tail_blk;
> +     if (tail_blk > head_blk) {
>               /*
>                * Perform recovery around the end of the physical log.
>                * When the head is not on the same cycle number as the tail,
> -              * we can't do a sequential recovery as above.
> +              * we can't do a sequential recovery.
>                */
> -             blk_no = tail_blk;
>               while (blk_no < log->l_logBBsize) {
>                       /*
>                        * Check for header wrapping around physical end-of-log
> @@ -4280,34 +4252,35 @@ xlog_do_recovery_pass(
>  
>               ASSERT(blk_no >= log->l_logBBsize);
>               blk_no -= log->l_logBBsize;
> +     }
>  
> -             /* read first part of physical log */
> -             while (blk_no < head_blk) {
> -                     error = xlog_bread(log, blk_no, hblks, hbp, &offset);
> -                     if (error)
> -                             goto bread_err2;
> +     /* read first part of physical log */
> +     while (blk_no < head_blk) {
> +             error = xlog_bread(log, blk_no, hblks, hbp, &offset);
> +             if (error)
> +                     goto bread_err2;
>  
> -                     rhead = (xlog_rec_header_t *)offset;
> -                     error = xlog_valid_rec_header(log, rhead, blk_no);
> -                     if (error)
> -                             goto bread_err2;
> +             rhead = (xlog_rec_header_t *)offset;
> +             error = xlog_valid_rec_header(log, rhead, blk_no);
> +             if (error)
> +                     goto bread_err2;
>  
> -                     bblks = (int)BTOBB(be32_to_cpu(rhead->h_len));
> -                     error = xlog_bread(log, blk_no+hblks, bblks, dbp,
> -                                        &offset);
> -                     if (error)
> -                             goto bread_err2;
> +             /* blocks in data section */
> +             bblks = (int)BTOBB(be32_to_cpu(rhead->h_len));
> +             error = xlog_bread(log, blk_no+hblks, bblks, dbp,
> +                                &offset);
> +             if (error)
> +                     goto bread_err2;
>  
> -                     error = xlog_unpack_data(rhead, offset, log);
> -                     if (error)
> -                             goto bread_err2;
> +             error = xlog_unpack_data(rhead, offset, log);
> +             if (error)
> +                     goto bread_err2;
>  
> -                     error = xlog_recover_process_data(log, rhash,
> -                                                     rhead, offset, pass);
> -                     if (error)
> -                             goto bread_err2;
> -                     blk_no += bblks + hblks;
> -             }
> +             error = xlog_recover_process_data(log, rhash,
> +                                             rhead, offset, pass);
> +             if (error)
> +                     goto bread_err2;
> +             blk_no += bblks + hblks;
>       }
>  
>   bread_err2:
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>