| Message ID | 165102073482.3922658.3874181264513799865.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: fix reflink inefficiencies | expand |
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>
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. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
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)); + + resp = (struct xfs_trans_res *)&resv; + end_resp = (struct xfs_trans_res *)(&resv + 1); + for (i = 0; resp < end_resp; i++, resp++) { int tmp = resp->tr_logcount > 1 ? resp->tr_logres * resp->tr_logcount : resp->tr_logres; + + trace_xfs_trans_resv_calc_minlogsize(mp, i, resp); if (log_space < tmp) { log_space = tmp; *max_resp = *resp; /* struct copy */ @@ -66,7 +73,7 @@ xfs_log_get_max_trans_res( } if (attr_space > log_space) { - *max_resp = M_RES(mp)->tr_attrsetm; /* struct copy */ + *max_resp = resv.tr_attrsetm; /* struct copy */ max_resp->tr_logres = attr_space; } } diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 91b916e82364..9110bb5dd866 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -3500,7 +3500,7 @@ DEFINE_GETFSMAP_EVENT(xfs_getfsmap_low_key); DEFINE_GETFSMAP_EVENT(xfs_getfsmap_high_key); DEFINE_GETFSMAP_EVENT(xfs_getfsmap_mapping); -TRACE_EVENT(xfs_trans_resv_calc, +DECLARE_EVENT_CLASS(xfs_trans_resv_class, TP_PROTO(struct xfs_mount *mp, unsigned int type, struct xfs_trans_res *res), TP_ARGS(mp, type, res), @@ -3524,7 +3524,15 @@ TRACE_EVENT(xfs_trans_resv_calc, __entry->logres, __entry->logcount, __entry->logflags) -); +) + +#define DEFINE_TRANS_RESV_EVENT(name) \ +DEFINE_EVENT(xfs_trans_resv_class, name, \ + TP_PROTO(struct xfs_mount *mp, unsigned int type, \ + struct xfs_trans_res *res), \ + TP_ARGS(mp, type, res)) +DEFINE_TRANS_RESV_EVENT(xfs_trans_resv_calc); +DEFINE_TRANS_RESV_EVENT(xfs_trans_resv_calc_minlogsize); DECLARE_EVENT_CLASS(xfs_trans_class, TP_PROTO(struct xfs_trans *tp, unsigned long caller_ip),