On Thu, Apr 25, 2024 at 06:12:23AM -0700, Christoph Hellwig wrote:
> This triggers the workaround for really old xfsprogs putting in a
> bogus h_size:
>
> [ 12.101992] XFS (loop0): invalid iclog size (0 bytes), using lsunit (65536 bytes)
>
> but then calculates the log recovery buffer size based on the actual
> on-disk h_size value. The patch below open codes xlog_logrec_hblks and
> fixes this particular reproducer. But I wonder if we should limit the
> workaround. Brian, you don't happpen to remember how old xfsprogs had
> to be to require your workaround (commit a70f9fe52daa8)?
>
No, but a little digging turns up xfsprogs commit 20fbd4593ff2 ("libxfs:
format the log with valid log record headers"), which I think is what
you're looking for..? That went in around v4.5 or so, so I suppose
anything earlier than that is affected.
Brian
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index b445e8ce4a7d21..b3ea546508dc93 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2999,7 +2999,7 @@ xlog_do_recovery_pass(
> int error = 0, h_size, h_len;
> int error2 = 0;
> int bblks, split_bblks;
> - int hblks, split_hblks, wrapped_hblks;
> + int hblks = 1, split_hblks, wrapped_hblks;
> int i;
> struct hlist_head rhash[XLOG_RHASH_SIZE];
> LIST_HEAD (buffer_list);
> @@ -3055,14 +3055,16 @@ xlog_do_recovery_pass(
> if (error)
> goto bread_err1;
>
> - hblks = xlog_logrec_hblks(log, rhead);
> - if (hblks != 1) {
> - kvfree(hbp);
> - hbp = xlog_alloc_buffer(log, hblks);
> + if ((rhead->h_version & cpu_to_be32(XLOG_VERSION_2)) &&
> + h_size > XLOG_HEADER_CYCLE_SIZE) {
> + hblks = DIV_ROUND_UP(h_size, XLOG_HEADER_CYCLE_SIZE);
> + if (hblks > 1) {
> + kvfree(hbp);
> + hbp = xlog_alloc_buffer(log, hblks);
> + }
> }
> } else {
> ASSERT(log->l_sectBBsize == 1);
> - hblks = 1;
> hbp = xlog_alloc_buffer(log, 1);
> h_size = XLOG_BIG_RECORD_BSIZE;
> }
>
