This looks very nice, but a bunch of minor comments:
> This is silly and expensive. The AIL is perfectly capable of
> calculating the push target itself, and it will always be running
> when the AIL contains objects.
>
> Modify the AIL to calculate it's 25% push target before it starts a
> push using the same reserve grant head based calculation as is
> currently used, and remove all the places where we ask the AIL to
> push to a new 25% free target.
>
I suspect some of these "the AIL" to xfsaild or te AIL pusher for
the sentences to make sense.
> +#include "xfs_trans_priv.h"
> +#include "xfs_log_priv.h"
> - threshold_lsn = xlog_grant_push_threshold(log, 0);
> + threshold_lsn = xfs_ail_push_target(log->l_ailp);
Should xfs_ail_push_target go into xfs_log.h instead of xfs_log_priv.h?
> +int xlog_space_left(struct xlog *log, atomic64_t *head);
Nit: odd formatting with the tab before the *log.
> +xfs_lsn_t
> +__xfs_ail_push_target(
> + struct xfs_ail *ailp)
> +{
> + struct xlog *log = ailp->ail_log;
> + xfs_lsn_t threshold_lsn = 0;
> + xfs_lsn_t last_sync_lsn;
> + int free_blocks;
> + int free_bytes;
> + int threshold_block;
> + int threshold_cycle;
> + int free_threshold;
This culd use a:
lockdep_assert_held(&ailp->ail_lock);
to document the locking assumptions.
> + free_bytes = xlog_space_left(log, &log->l_reserve_head.grant);
> + free_blocks = BTOBBT(free_bytes);
> +
> + /*
> + * Set the threshold for the minimum number of free blocks in the
> + * log to the maximum of what the caller needs, one quarter of the
> + * log, and 256 blocks.
> + */
> + free_threshold = log->l_logBBsize >> 2;
> + if (free_blocks >= free_threshold)
Nit: free_block is only used once, so there might not be much in a point
in keeping it as a logcal variable.
> +static inline void xfs_ail_push(struct xfs_ail *ailp)
> +{
> + wake_up_process(ailp->ail_task);
> +}
> +
> +static inline void xfs_ail_push_all(struct xfs_ail *ailp)
> +{
> + if (!test_and_set_bit(XFS_AIL_OPSTATE_PUSH_ALL, &ailp->ail_opstate))
> + xfs_ail_push(ailp);
> +}
> +
> +xfs_lsn_t __xfs_ail_push_target(struct xfs_ail *ailp);
> +static inline xfs_lsn_t xfs_ail_push_target(struct xfs_ail *ailp)
> +{
> + xfs_lsn_t lsn;
> +
> + spin_lock(&ailp->ail_lock);
> + lsn = __xfs_ail_push_target(ailp);
> + spin_unlock(&ailp->ail_lock);
> + return lsn;
> +}
Is there really any point in micro-optimizing these as inlines in
a header?
