On Tue, Jun 27, 2017 at 10:40:33AM -0400, Brian Foster wrote:
> The high-level log recovery algorithm consists of two loops that
> walk the physical log and process log records from the tail to the
> head. The first loop handles the case where the tail is beyond the
> head and processes records up to the end of the physical log. The
> subsequent loop processes records from the beginning of the physical
> log to the head.
>
> Because log records can wrap around the end of the physical log, the
> first loop mentioned above must handle this case appropriately.
> Records are processed from in-core buffers, which means that this
> algorithm must split the reads of such records into two partial
> I/Os: 1.) from the beginning of the record to the end of the log and
> 2.) from the beginning of the log to the end of the record. This is
> further complicated by the fact that the log record header and log
> record data are read into independent buffers.
>
> The current handling of each buffer correctly splits the reads when
> either the header or data starts before the end of the log and wraps
> around the end. The data read does not correctly handle the case
> where the prior header read wrapped or ends on the physical log end
> boundary. blk_no is incremented to or beyond the log end after the
> header read to point to the record data, but the split data read
> logic triggers, attempts to read from an invalid log block and
> ultimately causes log recovery to fail. This can be reproduced
> fairly reliably via xfstests tests generic/047 and generic/388 with
> large iclog sizes (256k) and small (10M) logs.
>
> If the record header read has pushed beyond the end of the physical
> log, the subsequent data read is actually contiguous. Update the
> data read logic to detect the case where blk_no has wrapped, mod it
> against the log size to read from the correct address and issue one
> contiguous read for the log data buffer. The log record is processed
> as normal from the buffer(s), the loop exits after the current
> iteration and the subsequent loop picks up with the first new record
> after the start of the log.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/xfs_log_recover.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index b6a40bd..9efcd12 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -5371,10 +5371,20 @@ xlog_do_recovery_pass(
> bblks = (int)BTOBB(be32_to_cpu(rhead->h_len));
> blk_no += hblks;
>
> - /* Read in data for log record */
> - if (blk_no + bblks <= log->l_logBBsize) {
> - error = xlog_bread(log, blk_no, bblks, dbp,
> - &offset);
> + /*
> + * Read the log record data in multiple reads if it
> + * wraps around the end of the log. Note that if the
> + * header already wrapped, blk_no could point past the
> + * end of the log. The record data is contiguous in
> + * that case.
> + */
> + if (blk_no + bblks <= log->l_logBBsize ||
> + blk_no >= log->l_logBBsize) {
> + /* mod blk_no in case the header wrapped and
> + * pushed it beyond the end of the log */
> + error = xlog_bread(log,
> + blk_no % log->l_logBBsize,
I /think/ this is ok, though isn't this 64-bit division? ^
--D
> + bblks, dbp, &offset);
> if (error)
> goto bread_err2;
> } else {
> --
> 2.7.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
