Add xlog_grant_head_check() to replace sections of
xlog_regrant_write_log_space() and xlog_grant_log_space().
On Mon, Dec 12, 2011 at 09:13:58AM -0500, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> ---
> fs/xfs/xfs_log.c | 133 ++++++++++++++++++++++++-------------------------------
> 1 file changed, 60 insertions(+), 73 deletions(-)
>
> Index: xfs/fs/xfs/xfs_log.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log.c 2011-12-11 21:32:08.763640044 +0100
> +++ xfs/fs/xfs/xfs_log.c 2011-12-11 21:36:10.395664344 +0100
> @@ -245,6 +245,60 @@ shutdown:
> return XFS_ERROR(EIO);
> }
>
> +/*
> + * Atomically get the log space required for a log ticket.
> + *
> + * Once a ticket gets put onto head->waiters, it will only return after the
> + * needed reservation is satisfied.
> + *
> + * This function is structured so that it has a lock free fast path. This is
> + * necessary because every new transaction reservation will come through this
> + * path. Hence any lock will be globally hot if we take it unconditionally on
> + * every pass.
> + *
> + * As tickets are only ever moved on and off head->waiters under head->lock, we
> + * only need to take that lock if we are going to add the ticket to the queue
> + * and sleep. We can avoid taking the lock if the ticket was never added to
> + * head->waiters because the t_queue list head will be empty and we hold the
> + * only reference to it so it can safely be checked unlocked.
> + */
> +STATIC int
> +xlog_grant_head_check(
> + struct log *log,
> + struct xlog_grant_head *head,
> + struct xlog_ticket *tic,
> + int *need_bytes)
> +{
> + int free_bytes;
> + int error = 0;
> +
> + ASSERT(!(log->l_flags & XLOG_ACTIVE_RECOVERY));
> +
> + /*
> + * If there are other waiters on the queue then give them a chance at
> + * logspace before us. Wake up the first waiters, if we do not wake
> + * up all the waiters then go to sleep waiting for more free space,
> + * otherwise try to get some space for this transaction.
> + */
> + *need_bytes = xlog_ticket_reservation(log, head, tic);
> + free_bytes = xlog_space_left(log, &head->grant);
> + if (!list_empty_careful(&head->waiters)) {
> + spin_lock(&head->lock);
> + if (!xlog_grant_head_wake(log, head, &free_bytes) ||
> + free_bytes < *need_bytes) {
> + error = xlog_grant_head_wait(log, head, tic,
> + *need_bytes);
> + }
> + spin_unlock(&head->lock);
> + } else if (free_bytes < *need_bytes) {
> + spin_lock(&head->lock);
> + error = xlog_grant_head_wait(log, head, tic, *need_bytes);
> + spin_unlock(&head->lock);
> + }
> +
> + return error;
> +}
> +
> static void
> xlog_tic_reset_res(xlog_ticket_t *tic)
> {
> @@ -2511,59 +2565,18 @@ restart:
> return 0;
> } /* xlog_state_get_iclog_space */
>
> -/*
> - * Atomically get the log space required for a log ticket.
> - *
> - * Once a ticket gets put onto the reserveq, it will only return after the
> - * needed reservation is satisfied.
> - *
> - * This function is structured so that it has a lock free fast path. This is
> - * necessary because every new transaction reservation will come through this
> - * path. Hence any lock will be globally hot if we take it unconditionally on
> - * every pass.
> - *
> - * As tickets are only ever moved on and off the l_reserve.waiters under the
> - * l_reserve.lock, we only need to take that lock if we are going to add
> - * the ticket to the queue and sleep. We can avoid taking the lock if the ticket
> - * was never added to the reserveq because the t_queue list head will be empty
> - * and we hold the only reference to it so it can safely be checked unlocked.
> - */
> STATIC int
> xlog_grant_log_space(
> struct log *log,
> struct xlog_ticket *tic)
> {
> - int free_bytes, need_bytes;
> + int need_bytes;
> int error = 0;
>
> - ASSERT(!(log->l_flags & XLOG_ACTIVE_RECOVERY));
> -
> trace_xfs_log_grant_enter(log, tic);
>
> - /*
> - * If there are other waiters on the queue then give them a chance at
> - * logspace before us. Wake up the first waiters, if we do not wake
> - * up all the waiters then go to sleep waiting for more free space,
> - * otherwise try to get some space for this transaction.
> - */
> - need_bytes = tic->t_unit_res;
> - if (tic->t_flags & XFS_LOG_PERM_RESERV)
> - need_bytes *= tic->t_ocnt;
^
Here the calculation was done with tic->t_ocnt.
You don't see it in this patch, but xlog_ticket_reservation is using
t_cnt. I haven't looked into which is correct.
Other than that this patch looks good to me.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
