| Message ID | 20200406123632.20873-4-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: automatic relogging experiment | expand |
On 4/6/20 5:36 AM, Brian Foster wrote: > Every transaction reservation includes runtime overhead on top of > the reservation calculated in the struct xfs_trans_res. This > overhead is required for things like the CIL context ticket, log > headers, etc., that are stolen from individual transactions. Since > reservation for the relog transaction is entirely contributed by > regular transactions, this runtime reservation overhead must be > contributed as well. This means that a transaction that relogs one > or more items must include overhead for the current transaction as > well as for the relog transaction. > > Define a new transaction flag to indicate that a transaction is > relog enabled. Plumb this state down to the log ticket allocation > and use it to bump the worst case overhead included in the > transaction. The overhead will eventually be transferred to the > relog system as needed for individual log items. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- > fs/xfs/libxfs/xfs_shared.h | 1 + > fs/xfs/xfs_log.c | 12 +++++++++--- > fs/xfs/xfs_log.h | 3 ++- > fs/xfs/xfs_log_cil.c | 2 +- > fs/xfs/xfs_log_priv.h | 1 + > fs/xfs/xfs_trans.c | 3 ++- > 6 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h > index c45acbd3add9..1ede1e720a5c 100644 > --- a/fs/xfs/libxfs/xfs_shared.h > +++ b/fs/xfs/libxfs/xfs_shared.h > @@ -65,6 +65,7 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, > #define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ > #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ > #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ > +#define XFS_TRANS_RELOG 0x80 /* requires extra relog overhead */ > /* > * LOWMODE is used by the allocator to activate the lowspace algorithm - when > * free space is running low the extent allocator may choose to allocate an > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index d6b63490a78b..b55abde6c142 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -418,7 +418,8 @@ xfs_log_reserve( > int cnt, > struct xlog_ticket **ticp, > uint8_t client, > - bool permanent) > + bool permanent, > + bool relog) > { > struct xlog *log = mp->m_log; > struct xlog_ticket *tic; > @@ -433,7 +434,8 @@ xfs_log_reserve( > XFS_STATS_INC(mp, xs_try_logspace); > > ASSERT(*ticp == NULL); > - tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0); > + tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, relog, > + 0); > *ticp = tic; > > xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt > @@ -831,7 +833,7 @@ xlog_unmount_write( > uint flags = XLOG_UNMOUNT_TRANS; > int error; > > - error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, 0); > + error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, false, false); > if (error) > goto out_err; > > @@ -3421,6 +3423,7 @@ xlog_ticket_alloc( > int cnt, > char client, > bool permanent, > + bool relog, > xfs_km_flags_t alloc_flags) I notice this routine already has a flag param. Wondering if it would make sense to have a KM_RELOG flag instead of an extra bool param? It would just be one less thing to pass around. Other than that, it seems straight forward. Reviewed-by: Allison Collins <allison.henderson@oracle.com> > { > struct xlog_ticket *tic; > @@ -3431,6 +3434,9 @@ xlog_ticket_alloc( > return NULL; > > unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes); > + /* double the overhead for the relog transaction */ > + if (relog) > + unit_res += (unit_res - unit_bytes); > > atomic_set(&tic->t_ref, 1); > tic->t_task = current; > diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h > index 6d2f30f42245..f1089a4b299c 100644 > --- a/fs/xfs/xfs_log.h > +++ b/fs/xfs/xfs_log.h > @@ -123,7 +123,8 @@ int xfs_log_reserve(struct xfs_mount *mp, > int count, > struct xlog_ticket **ticket, > uint8_t clientid, > - bool permanent); > + bool permanent, > + bool relog); > int xfs_log_regrant(struct xfs_mount *mp, struct xlog_ticket *tic); > void xfs_log_ungrant_bytes(struct xfs_mount *mp, int bytes); > void xfs_log_unmount(struct xfs_mount *mp); > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c > index b43f0e8f43f2..1c48e95402aa 100644 > --- a/fs/xfs/xfs_log_cil.c > +++ b/fs/xfs/xfs_log_cil.c > @@ -37,7 +37,7 @@ xlog_cil_ticket_alloc( > { > struct xlog_ticket *tic; > > - tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0, > + tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, false, false, > KM_NOFS); > > /* > diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h > index ec22c7a3867f..08d8ff9bce1a 100644 > --- a/fs/xfs/xfs_log_priv.h > +++ b/fs/xfs/xfs_log_priv.h > @@ -465,6 +465,7 @@ xlog_ticket_alloc( > int count, > char client, > bool permanent, > + bool relog, > xfs_km_flags_t alloc_flags); > > > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c > index 4fbe11485bbb..1b25980315bd 100644 > --- a/fs/xfs/xfs_trans.c > +++ b/fs/xfs/xfs_trans.c > @@ -177,6 +177,7 @@ xfs_trans_reserve( > */ > if (resp->tr_logres > 0) { > bool permanent = false; > + bool relog = (tp->t_flags & XFS_TRANS_RELOG); > > ASSERT(tp->t_log_res == 0 || > tp->t_log_res == resp->tr_logres); > @@ -199,7 +200,7 @@ xfs_trans_reserve( > resp->tr_logres, > resp->tr_logcount, > &tp->t_ticket, XFS_TRANSACTION, > - permanent); > + permanent, relog); > } > > if (error) >
On Tue, Apr 07, 2020 at 04:04:43PM -0700, Allison Collins wrote: > > > On 4/6/20 5:36 AM, Brian Foster wrote: > > Every transaction reservation includes runtime overhead on top of > > the reservation calculated in the struct xfs_trans_res. This > > overhead is required for things like the CIL context ticket, log > > headers, etc., that are stolen from individual transactions. Since > > reservation for the relog transaction is entirely contributed by > > regular transactions, this runtime reservation overhead must be > > contributed as well. This means that a transaction that relogs one > > or more items must include overhead for the current transaction as > > well as for the relog transaction. > > > > Define a new transaction flag to indicate that a transaction is > > relog enabled. Plumb this state down to the log ticket allocation > > and use it to bump the worst case overhead included in the > > transaction. The overhead will eventually be transferred to the > > relog system as needed for individual log items. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > --- > > fs/xfs/libxfs/xfs_shared.h | 1 + > > fs/xfs/xfs_log.c | 12 +++++++++--- > > fs/xfs/xfs_log.h | 3 ++- > > fs/xfs/xfs_log_cil.c | 2 +- > > fs/xfs/xfs_log_priv.h | 1 + > > fs/xfs/xfs_trans.c | 3 ++- > > 6 files changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h > > index c45acbd3add9..1ede1e720a5c 100644 > > --- a/fs/xfs/libxfs/xfs_shared.h > > +++ b/fs/xfs/libxfs/xfs_shared.h > > @@ -65,6 +65,7 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, > > #define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ > > #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ > > #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ > > +#define XFS_TRANS_RELOG 0x80 /* requires extra relog overhead */ > > /* > > * LOWMODE is used by the allocator to activate the lowspace algorithm - when > > * free space is running low the extent allocator may choose to allocate an > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > > index d6b63490a78b..b55abde6c142 100644 > > --- a/fs/xfs/xfs_log.c > > +++ b/fs/xfs/xfs_log.c > > @@ -418,7 +418,8 @@ xfs_log_reserve( > > int cnt, > > struct xlog_ticket **ticp, > > uint8_t client, > > - bool permanent) > > + bool permanent, > > + bool relog) > > { > > struct xlog *log = mp->m_log; > > struct xlog_ticket *tic; > > @@ -433,7 +434,8 @@ xfs_log_reserve( > > XFS_STATS_INC(mp, xs_try_logspace); > > ASSERT(*ticp == NULL); > > - tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0); > > + tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, relog, > > + 0); > > *ticp = tic; > > xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt > > @@ -831,7 +833,7 @@ xlog_unmount_write( > > uint flags = XLOG_UNMOUNT_TRANS; > > int error; > > - error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, 0); > > + error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, false, false); > > if (error) > > goto out_err; > > @@ -3421,6 +3423,7 @@ xlog_ticket_alloc( > > int cnt, > > char client, > > bool permanent, > > + bool relog, > > xfs_km_flags_t alloc_flags) > I notice this routine already has a flag param. Wondering if it would make > sense to have a KM_RELOG flag instead of an extra bool param? It would just > be one less thing to pass around. Other than that, it seems straight > forward. > In the xlog_ticket_alloc() case, those are actually memory allocation flags (as opposed to transaction flags used in xfs_trans_alloc()). The former includes things like KM_NOFS, etc., that translate to GFP_* allocation flags. So I don't think it's appropriate to stash a transaction or ticket oriented flag there. I did halfway expect some feedback wrt to XFS_TRANS_RELOG because it technically could be buried in the reservation structure itself, similar to how permanent state is handled (though we'd still need to pass the boolean around a bit). I opted to introduce XFS_TRANS_RELOG here instead because I need to use that for random buffer relogging later in the series, so it accomplished both at least for the time being. For a non-RFC series I could look into moving that flag into the reservation, relegating TRANS_RELOG to debug mode and letting the test code use it from there. > Reviewed-by: Allison Collins <allison.henderson@oracle.com> > Thanks! Brian > > { > > struct xlog_ticket *tic; > > @@ -3431,6 +3434,9 @@ xlog_ticket_alloc( > > return NULL; > > unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes); > > + /* double the overhead for the relog transaction */ > > + if (relog) > > + unit_res += (unit_res - unit_bytes); > > atomic_set(&tic->t_ref, 1); > > tic->t_task = current; > > diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h > > index 6d2f30f42245..f1089a4b299c 100644 > > --- a/fs/xfs/xfs_log.h > > +++ b/fs/xfs/xfs_log.h > > @@ -123,7 +123,8 @@ int xfs_log_reserve(struct xfs_mount *mp, > > int count, > > struct xlog_ticket **ticket, > > uint8_t clientid, > > - bool permanent); > > + bool permanent, > > + bool relog); > > int xfs_log_regrant(struct xfs_mount *mp, struct xlog_ticket *tic); > > void xfs_log_ungrant_bytes(struct xfs_mount *mp, int bytes); > > void xfs_log_unmount(struct xfs_mount *mp); > > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c > > index b43f0e8f43f2..1c48e95402aa 100644 > > --- a/fs/xfs/xfs_log_cil.c > > +++ b/fs/xfs/xfs_log_cil.c > > @@ -37,7 +37,7 @@ xlog_cil_ticket_alloc( > > { > > struct xlog_ticket *tic; > > - tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0, > > + tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, false, false, > > KM_NOFS); > > /* > > diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h > > index ec22c7a3867f..08d8ff9bce1a 100644 > > --- a/fs/xfs/xfs_log_priv.h > > +++ b/fs/xfs/xfs_log_priv.h > > @@ -465,6 +465,7 @@ xlog_ticket_alloc( > > int count, > > char client, > > bool permanent, > > + bool relog, > > xfs_km_flags_t alloc_flags); > > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c > > index 4fbe11485bbb..1b25980315bd 100644 > > --- a/fs/xfs/xfs_trans.c > > +++ b/fs/xfs/xfs_trans.c > > @@ -177,6 +177,7 @@ xfs_trans_reserve( > > */ > > if (resp->tr_logres > 0) { > > bool permanent = false; > > + bool relog = (tp->t_flags & XFS_TRANS_RELOG); > > ASSERT(tp->t_log_res == 0 || > > tp->t_log_res == resp->tr_logres); > > @@ -199,7 +200,7 @@ xfs_trans_reserve( > > resp->tr_logres, > > resp->tr_logcount, > > &tp->t_ticket, XFS_TRANSACTION, > > - permanent); > > + permanent, relog); > > } > > if (error) > > >
diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h index c45acbd3add9..1ede1e720a5c 100644 --- a/fs/xfs/libxfs/xfs_shared.h +++ b/fs/xfs/libxfs/xfs_shared.h @@ -65,6 +65,7 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp, #define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */ #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */ #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */ +#define XFS_TRANS_RELOG 0x80 /* requires extra relog overhead */ /* * LOWMODE is used by the allocator to activate the lowspace algorithm - when * free space is running low the extent allocator may choose to allocate an diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index d6b63490a78b..b55abde6c142 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -418,7 +418,8 @@ xfs_log_reserve( int cnt, struct xlog_ticket **ticp, uint8_t client, - bool permanent) + bool permanent, + bool relog) { struct xlog *log = mp->m_log; struct xlog_ticket *tic; @@ -433,7 +434,8 @@ xfs_log_reserve( XFS_STATS_INC(mp, xs_try_logspace); ASSERT(*ticp == NULL); - tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0); + tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, relog, + 0); *ticp = tic; xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt @@ -831,7 +833,7 @@ xlog_unmount_write( uint flags = XLOG_UNMOUNT_TRANS; int error; - error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, 0); + error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, false, false); if (error) goto out_err; @@ -3421,6 +3423,7 @@ xlog_ticket_alloc( int cnt, char client, bool permanent, + bool relog, xfs_km_flags_t alloc_flags) { struct xlog_ticket *tic; @@ -3431,6 +3434,9 @@ xlog_ticket_alloc( return NULL; unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes); + /* double the overhead for the relog transaction */ + if (relog) + unit_res += (unit_res - unit_bytes); atomic_set(&tic->t_ref, 1); tic->t_task = current; diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h index 6d2f30f42245..f1089a4b299c 100644 --- a/fs/xfs/xfs_log.h +++ b/fs/xfs/xfs_log.h @@ -123,7 +123,8 @@ int xfs_log_reserve(struct xfs_mount *mp, int count, struct xlog_ticket **ticket, uint8_t clientid, - bool permanent); + bool permanent, + bool relog); int xfs_log_regrant(struct xfs_mount *mp, struct xlog_ticket *tic); void xfs_log_ungrant_bytes(struct xfs_mount *mp, int bytes); void xfs_log_unmount(struct xfs_mount *mp); diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c index b43f0e8f43f2..1c48e95402aa 100644 --- a/fs/xfs/xfs_log_cil.c +++ b/fs/xfs/xfs_log_cil.c @@ -37,7 +37,7 @@ xlog_cil_ticket_alloc( { struct xlog_ticket *tic; - tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0, + tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, false, false, KM_NOFS); /* diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h index ec22c7a3867f..08d8ff9bce1a 100644 --- a/fs/xfs/xfs_log_priv.h +++ b/fs/xfs/xfs_log_priv.h @@ -465,6 +465,7 @@ xlog_ticket_alloc( int count, char client, bool permanent, + bool relog, xfs_km_flags_t alloc_flags); diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 4fbe11485bbb..1b25980315bd 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -177,6 +177,7 @@ xfs_trans_reserve( */ if (resp->tr_logres > 0) { bool permanent = false; + bool relog = (tp->t_flags & XFS_TRANS_RELOG); ASSERT(tp->t_log_res == 0 || tp->t_log_res == resp->tr_logres); @@ -199,7 +200,7 @@ xfs_trans_reserve( resp->tr_logres, resp->tr_logcount, &tp->t_ticket, XFS_TRANSACTION, - permanent); + permanent, relog); } if (error)
Every transaction reservation includes runtime overhead on top of the reservation calculated in the struct xfs_trans_res. This overhead is required for things like the CIL context ticket, log headers, etc., that are stolen from individual transactions. Since reservation for the relog transaction is entirely contributed by regular transactions, this runtime reservation overhead must be contributed as well. This means that a transaction that relogs one or more items must include overhead for the current transaction as well as for the relog transaction. Define a new transaction flag to indicate that a transaction is relog enabled. Plumb this state down to the log ticket allocation and use it to bump the worst case overhead included in the transaction. The overhead will eventually be transferred to the relog system as needed for individual log items. Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/libxfs/xfs_shared.h | 1 + fs/xfs/xfs_log.c | 12 +++++++++--- fs/xfs/xfs_log.h | 3 ++- fs/xfs/xfs_log_cil.c | 2 +- fs/xfs/xfs_log_priv.h | 1 + fs/xfs/xfs_trans.c | 3 ++- 6 files changed, 16 insertions(+), 6 deletions(-)