Re: [PATCH 4/9] xfs: create shadow transaction reservations for computing minimum log size

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 26, 2022 at 05:52:14PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Every time someone changes the transaction reservation sizes, they
> introduce potential compatibility problems if the changes affect the
> minimum log size that we validate at mount time.  If the minimum log
> size gets larger (which should be avoided because doing so presents a
> serious risk of log livelock), filesystems created with old mkfs will
> not mount on a newer kernel; if the minimum size shrinks, filesystems
> created with newer mkfs will not mount on older kernels.
> 
> Therefore, enable the creation of a shadow log reservation structure
> where we can "undo" the effects of tweaks when computing minimum log
> sizes.  These shadow reservations should never be used in practice, but
> they insulate us from perturbations in minimum log size.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/libxfs/xfs_log_rlimit.c |   15 +++++++++++----
>  fs/xfs/xfs_trace.h             |   12 ++++++++++--
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
> index 67798ff5e14e..4d04568ab07e 100644
> --- a/fs/xfs/libxfs/xfs_log_rlimit.c
> +++ b/fs/xfs/libxfs/xfs_log_rlimit.c
> @@ -14,6 +14,7 @@
>  #include "xfs_trans_space.h"
>  #include "xfs_da_btree.h"
>  #include "xfs_bmap_btree.h"
> +#include "xfs_trace.h"
>  
>  /*
>   * Calculate the maximum length in bytes that would be required for a local
> @@ -46,19 +47,25 @@ xfs_log_get_max_trans_res(
>  	struct xfs_mount	*mp,
>  	struct xfs_trans_res	*max_resp)
>  {
> +	struct xfs_trans_resv	resv;
>  	struct xfs_trans_res	*resp;
>  	struct xfs_trans_res	*end_resp;
> +	unsigned int		i;
>  	int			log_space = 0;
>  	int			attr_space;
>  
>  	attr_space = xfs_log_calc_max_attrsetm_res(mp);
>  
> -	resp = (struct xfs_trans_res *)M_RES(mp);
> -	end_resp = (struct xfs_trans_res *)(M_RES(mp) + 1);
> -	for (; resp < end_resp; resp++) {
> +	memcpy(&resv, M_RES(mp), sizeof(struct xfs_trans_resv));

Looks much nicer, but I had to read on further into the patchset
before it made sense. As it all ends up ok:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com