| Message ID | 20200227134321.7238-5-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: automatic relogging experiment | expand |
On 2/27/20 6:43 AM, Brian Foster wrote: > As implemented by the previous patch, relogging can be enabled on > any item via a relog enabled transaction (which holds a reference to > an active relog ticket). Add a couple log item flags to track relog > state of an arbitrary log item. The item holds a reference to the > global relog ticket when relogging is enabled and releases the > reference when relogging is disabled. > Alrighty, I think it's ok Reviewed-by: Allison Collins <allison.henderson@oracle.com> > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- > fs/xfs/xfs_trace.h | 2 ++ > fs/xfs/xfs_trans.c | 36 ++++++++++++++++++++++++++++++++++++ > fs/xfs/xfs_trans.h | 6 +++++- > fs/xfs/xfs_trans_priv.h | 2 ++ > 4 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > index a86be7f807ee..a066617ec54d 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -1063,6 +1063,8 @@ DEFINE_LOG_ITEM_EVENT(xfs_ail_push); > DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned); > DEFINE_LOG_ITEM_EVENT(xfs_ail_locked); > DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing); > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item); > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item_cancel); > > DECLARE_EVENT_CLASS(xfs_ail_class, > TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn), > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c > index 8ac05ed8deda..f7f2411ead4e 100644 > --- a/fs/xfs/xfs_trans.c > +++ b/fs/xfs/xfs_trans.c > @@ -778,6 +778,41 @@ xfs_trans_del_item( > list_del_init(&lip->li_trans); > } > > +void > +xfs_trans_relog_item( > + struct xfs_log_item *lip) > +{ > + if (!test_and_set_bit(XFS_LI_RELOG, &lip->li_flags)) { > + xfs_trans_ail_relog_get(lip->li_mountp); > + trace_xfs_relog_item(lip); > + } > +} > + > +void > +xfs_trans_relog_item_cancel( > + struct xfs_log_item *lip, > + bool drain) /* wait for relogging to cease */ > +{ > + struct xfs_mount *mp = lip->li_mountp; > + > + if (!test_and_clear_bit(XFS_LI_RELOG, &lip->li_flags)) > + return; > + xfs_trans_ail_relog_put(lip->li_mountp); > + trace_xfs_relog_item_cancel(lip); > + > + if (!drain) > + return; > + > + /* > + * Some operations might require relog activity to cease before they can > + * proceed. For example, an operation must wait before including a > + * non-lockable log item (i.e. intent) in another transaction. > + */ > + while (wait_on_bit_timeout(&lip->li_flags, XFS_LI_RELOGGED, > + TASK_UNINTERRUPTIBLE, HZ)) > + xfs_log_force(mp, XFS_LOG_SYNC); > +} > + > /* Detach and unlock all of the items in a transaction */ > static void > xfs_trans_free_items( > @@ -863,6 +898,7 @@ xfs_trans_committed_bulk( > > if (aborted) > set_bit(XFS_LI_ABORTED, &lip->li_flags); > + clear_and_wake_up_bit(XFS_LI_RELOGGED, &lip->li_flags); > > if (lip->li_ops->flags & XFS_ITEM_RELEASE_WHEN_COMMITTED) { > lip->li_ops->iop_release(lip); > diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h > index a032989943bd..fc4c25b6eee4 100644 > --- a/fs/xfs/xfs_trans.h > +++ b/fs/xfs/xfs_trans.h > @@ -59,12 +59,16 @@ struct xfs_log_item { > #define XFS_LI_ABORTED 1 > #define XFS_LI_FAILED 2 > #define XFS_LI_DIRTY 3 /* log item dirty in transaction */ > +#define XFS_LI_RELOG 4 /* automatically relog item */ > +#define XFS_LI_RELOGGED 5 /* item relogged (not committed) */ > > #define XFS_LI_FLAGS \ > { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \ > { (1 << XFS_LI_ABORTED), "ABORTED" }, \ > { (1 << XFS_LI_FAILED), "FAILED" }, \ > - { (1 << XFS_LI_DIRTY), "DIRTY" } > + { (1 << XFS_LI_DIRTY), "DIRTY" }, \ > + { (1 << XFS_LI_RELOG), "RELOG" }, \ > + { (1 << XFS_LI_RELOGGED), "RELOGGED" } > > struct xfs_item_ops { > unsigned flags; > diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h > index 839df6559b9f..d1edec1cb8ad 100644 > --- a/fs/xfs/xfs_trans_priv.h > +++ b/fs/xfs/xfs_trans_priv.h > @@ -16,6 +16,8 @@ struct xfs_log_vec; > void xfs_trans_init(struct xfs_mount *); > void xfs_trans_add_item(struct xfs_trans *, struct xfs_log_item *); > void xfs_trans_del_item(struct xfs_log_item *); > +void xfs_trans_relog_item(struct xfs_log_item *); > +void xfs_trans_relog_item_cancel(struct xfs_log_item *, bool); > void xfs_trans_unreserve_and_mod_sb(struct xfs_trans *tp); > > void xfs_trans_committed_bulk(struct xfs_ail *ailp, struct xfs_log_vec *lv, >
On Thu, Feb 27, 2020 at 08:43:16AM -0500, Brian Foster wrote: > As implemented by the previous patch, relogging can be enabled on > any item via a relog enabled transaction (which holds a reference to > an active relog ticket). Add a couple log item flags to track relog > state of an arbitrary log item. The item holds a reference to the > global relog ticket when relogging is enabled and releases the > reference when relogging is disabled. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- > fs/xfs/xfs_trace.h | 2 ++ > fs/xfs/xfs_trans.c | 36 ++++++++++++++++++++++++++++++++++++ > fs/xfs/xfs_trans.h | 6 +++++- > fs/xfs/xfs_trans_priv.h | 2 ++ > 4 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > index a86be7f807ee..a066617ec54d 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -1063,6 +1063,8 @@ DEFINE_LOG_ITEM_EVENT(xfs_ail_push); > DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned); > DEFINE_LOG_ITEM_EVENT(xfs_ail_locked); > DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing); > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item); > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item_cancel); > > DECLARE_EVENT_CLASS(xfs_ail_class, > TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn), > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c > index 8ac05ed8deda..f7f2411ead4e 100644 > --- a/fs/xfs/xfs_trans.c > +++ b/fs/xfs/xfs_trans.c > @@ -778,6 +778,41 @@ xfs_trans_del_item( > list_del_init(&lip->li_trans); > } > > +void > +xfs_trans_relog_item( > + struct xfs_log_item *lip) > +{ > + if (!test_and_set_bit(XFS_LI_RELOG, &lip->li_flags)) { > + xfs_trans_ail_relog_get(lip->li_mountp); > + trace_xfs_relog_item(lip); > + } What if xfs_trans_ail_relog_get() fails to get a reference here because there is no current ail relog ticket? Isn't the transaction it was reserved in required to be checked here for XFS_TRANS_RELOG being set? > +} > + > +void > +xfs_trans_relog_item_cancel( > + struct xfs_log_item *lip, > + bool drain) /* wait for relogging to cease */ > +{ > + struct xfs_mount *mp = lip->li_mountp; > + > + if (!test_and_clear_bit(XFS_LI_RELOG, &lip->li_flags)) > + return; > + xfs_trans_ail_relog_put(lip->li_mountp); > + trace_xfs_relog_item_cancel(lip); > + > + if (!drain) > + return; > + > + /* > + * Some operations might require relog activity to cease before they can > + * proceed. For example, an operation must wait before including a > + * non-lockable log item (i.e. intent) in another transaction. > + */ > + while (wait_on_bit_timeout(&lip->li_flags, XFS_LI_RELOGGED, > + TASK_UNINTERRUPTIBLE, HZ)) > + xfs_log_force(mp, XFS_LOG_SYNC); > +} What is a "cancel" operation? Is it something you do when cancelling a transaction (i.e. on operation failure) or is is something the final transaction does to remove the relog item from the AIL (i.e. part of the normal successful finish to a long running transaction)? > /* Detach and unlock all of the items in a transaction */ > static void > xfs_trans_free_items( > @@ -863,6 +898,7 @@ xfs_trans_committed_bulk( > > if (aborted) > set_bit(XFS_LI_ABORTED, &lip->li_flags); > + clear_and_wake_up_bit(XFS_LI_RELOGGED, &lip->li_flags); I don't know what the XFS_LI_RELOGGED flag really means in this patch because I don't know what sets it. Perhaps this would be better moved into the patch that first sets the RELOGGED flag? Cheers, Dave.
On Mon, Mar 02, 2020 at 04:58:37PM +1100, Dave Chinner wrote: > On Thu, Feb 27, 2020 at 08:43:16AM -0500, Brian Foster wrote: > > As implemented by the previous patch, relogging can be enabled on > > any item via a relog enabled transaction (which holds a reference to > > an active relog ticket). Add a couple log item flags to track relog > > state of an arbitrary log item. The item holds a reference to the > > global relog ticket when relogging is enabled and releases the > > reference when relogging is disabled. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > --- > > fs/xfs/xfs_trace.h | 2 ++ > > fs/xfs/xfs_trans.c | 36 ++++++++++++++++++++++++++++++++++++ > > fs/xfs/xfs_trans.h | 6 +++++- > > fs/xfs/xfs_trans_priv.h | 2 ++ > > 4 files changed, 45 insertions(+), 1 deletion(-) > > > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > > index a86be7f807ee..a066617ec54d 100644 > > --- a/fs/xfs/xfs_trace.h > > +++ b/fs/xfs/xfs_trace.h > > @@ -1063,6 +1063,8 @@ DEFINE_LOG_ITEM_EVENT(xfs_ail_push); > > DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned); > > DEFINE_LOG_ITEM_EVENT(xfs_ail_locked); > > DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing); > > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item); > > +DEFINE_LOG_ITEM_EVENT(xfs_relog_item_cancel); > > > > DECLARE_EVENT_CLASS(xfs_ail_class, > > TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn), > > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c > > index 8ac05ed8deda..f7f2411ead4e 100644 > > --- a/fs/xfs/xfs_trans.c > > +++ b/fs/xfs/xfs_trans.c > > @@ -778,6 +778,41 @@ xfs_trans_del_item( > > list_del_init(&lip->li_trans); > > } > > > > +void > > +xfs_trans_relog_item( > > + struct xfs_log_item *lip) > > +{ > > + if (!test_and_set_bit(XFS_LI_RELOG, &lip->li_flags)) { > > + xfs_trans_ail_relog_get(lip->li_mountp); > > + trace_xfs_relog_item(lip); > > + } > > What if xfs_trans_ail_relog_get() fails to get a reference here > because there is no current ail relog ticket? Isn't the transaction > it was reserved in required to be checked here for XFS_TRANS_RELOG > being set? > That shouldn't happen because XFS_TRANS_RELOG is required of the transaction, as you noted. Ideally this would at least have an assert. I'm guessing I didn't do that simply because there was no other reason to pass the transaction into this function. I could clean this up, but much of this might go away if the reservation model changes such that XFS_TRANS_RELOG goes away. > > +} > > + > > +void > > +xfs_trans_relog_item_cancel( > > + struct xfs_log_item *lip, > > + bool drain) /* wait for relogging to cease */ > > +{ > > + struct xfs_mount *mp = lip->li_mountp; > > + > > + if (!test_and_clear_bit(XFS_LI_RELOG, &lip->li_flags)) > > + return; > > + xfs_trans_ail_relog_put(lip->li_mountp); > > + trace_xfs_relog_item_cancel(lip); > > + > > + if (!drain) > > + return; > > + > > + /* > > + * Some operations might require relog activity to cease before they can > > + * proceed. For example, an operation must wait before including a > > + * non-lockable log item (i.e. intent) in another transaction. > > + */ > > + while (wait_on_bit_timeout(&lip->li_flags, XFS_LI_RELOGGED, > > + TASK_UNINTERRUPTIBLE, HZ)) > > + xfs_log_force(mp, XFS_LOG_SYNC); > > +} > > What is a "cancel" operation? Is it something you do when cancelling > a transaction (i.e. on operation failure) or is is something the > final transaction does to remove the relog item from the AIL (i.e. > part of the normal successful finish to a long running transaction)? > This just means to cancel relogging on a log item. To cancel relogging only requires to clear the flag, so it doesn't require a transaction at all at the moment. The waiting bit is for callers (i.e. quotaoff) that might want to remove the item from the AIL after relogging is disabled. Without that, the item could still be in the CIL when the caller wants to remove it. > > > /* Detach and unlock all of the items in a transaction */ > > static void > > xfs_trans_free_items( > > @@ -863,6 +898,7 @@ xfs_trans_committed_bulk( > > > > if (aborted) > > set_bit(XFS_LI_ABORTED, &lip->li_flags); > > + clear_and_wake_up_bit(XFS_LI_RELOGGED, &lip->li_flags); > > I don't know what the XFS_LI_RELOGGED flag really means in this > patch because I don't know what sets it. Perhaps this would be > better moved into the patch that first sets the RELOGGED flag? > Hmm, yeah that's a bit of wart. It basically means that an item is queued for relog by the AIL (similar to an item added to the buffer writeback list, but not yet submitted). Perhaps RELOG_QUEUED would be a better name. It's included in this patch because it's used by xfs_trans_relog_item_cancel(), but I suppose that whole functional hunk could be added later once the flag is introduced properly. Brian > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index a86be7f807ee..a066617ec54d 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -1063,6 +1063,8 @@ DEFINE_LOG_ITEM_EVENT(xfs_ail_push); DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned); DEFINE_LOG_ITEM_EVENT(xfs_ail_locked); DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing); +DEFINE_LOG_ITEM_EVENT(xfs_relog_item); +DEFINE_LOG_ITEM_EVENT(xfs_relog_item_cancel); DECLARE_EVENT_CLASS(xfs_ail_class, TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn), diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c index 8ac05ed8deda..f7f2411ead4e 100644 --- a/fs/xfs/xfs_trans.c +++ b/fs/xfs/xfs_trans.c @@ -778,6 +778,41 @@ xfs_trans_del_item( list_del_init(&lip->li_trans); } +void +xfs_trans_relog_item( + struct xfs_log_item *lip) +{ + if (!test_and_set_bit(XFS_LI_RELOG, &lip->li_flags)) { + xfs_trans_ail_relog_get(lip->li_mountp); + trace_xfs_relog_item(lip); + } +} + +void +xfs_trans_relog_item_cancel( + struct xfs_log_item *lip, + bool drain) /* wait for relogging to cease */ +{ + struct xfs_mount *mp = lip->li_mountp; + + if (!test_and_clear_bit(XFS_LI_RELOG, &lip->li_flags)) + return; + xfs_trans_ail_relog_put(lip->li_mountp); + trace_xfs_relog_item_cancel(lip); + + if (!drain) + return; + + /* + * Some operations might require relog activity to cease before they can + * proceed. For example, an operation must wait before including a + * non-lockable log item (i.e. intent) in another transaction. + */ + while (wait_on_bit_timeout(&lip->li_flags, XFS_LI_RELOGGED, + TASK_UNINTERRUPTIBLE, HZ)) + xfs_log_force(mp, XFS_LOG_SYNC); +} + /* Detach and unlock all of the items in a transaction */ static void xfs_trans_free_items( @@ -863,6 +898,7 @@ xfs_trans_committed_bulk( if (aborted) set_bit(XFS_LI_ABORTED, &lip->li_flags); + clear_and_wake_up_bit(XFS_LI_RELOGGED, &lip->li_flags); if (lip->li_ops->flags & XFS_ITEM_RELEASE_WHEN_COMMITTED) { lip->li_ops->iop_release(lip); diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h index a032989943bd..fc4c25b6eee4 100644 --- a/fs/xfs/xfs_trans.h +++ b/fs/xfs/xfs_trans.h @@ -59,12 +59,16 @@ struct xfs_log_item { #define XFS_LI_ABORTED 1 #define XFS_LI_FAILED 2 #define XFS_LI_DIRTY 3 /* log item dirty in transaction */ +#define XFS_LI_RELOG 4 /* automatically relog item */ +#define XFS_LI_RELOGGED 5 /* item relogged (not committed) */ #define XFS_LI_FLAGS \ { (1 << XFS_LI_IN_AIL), "IN_AIL" }, \ { (1 << XFS_LI_ABORTED), "ABORTED" }, \ { (1 << XFS_LI_FAILED), "FAILED" }, \ - { (1 << XFS_LI_DIRTY), "DIRTY" } + { (1 << XFS_LI_DIRTY), "DIRTY" }, \ + { (1 << XFS_LI_RELOG), "RELOG" }, \ + { (1 << XFS_LI_RELOGGED), "RELOGGED" } struct xfs_item_ops { unsigned flags; diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h index 839df6559b9f..d1edec1cb8ad 100644 --- a/fs/xfs/xfs_trans_priv.h +++ b/fs/xfs/xfs_trans_priv.h @@ -16,6 +16,8 @@ struct xfs_log_vec; void xfs_trans_init(struct xfs_mount *); void xfs_trans_add_item(struct xfs_trans *, struct xfs_log_item *); void xfs_trans_del_item(struct xfs_log_item *); +void xfs_trans_relog_item(struct xfs_log_item *); +void xfs_trans_relog_item_cancel(struct xfs_log_item *, bool); void xfs_trans_unreserve_and_mod_sb(struct xfs_trans *tp); void xfs_trans_committed_bulk(struct xfs_ail *ailp, struct xfs_log_vec *lv,
As implemented by the previous patch, relogging can be enabled on any item via a relog enabled transaction (which holds a reference to an active relog ticket). Add a couple log item flags to track relog state of an arbitrary log item. The item holds a reference to the global relog ticket when relogging is enabled and releases the reference when relogging is disabled. Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/xfs_trace.h | 2 ++ fs/xfs/xfs_trans.c | 36 ++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_trans.h | 6 +++++- fs/xfs/xfs_trans_priv.h | 2 ++ 4 files changed, 45 insertions(+), 1 deletion(-)