xfs
[Top] [All Lists]

Re: [PATCH 9/9] xfs: log ticket reservation underestimates the number of

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 9/9] xfs: log ticket reservation underestimates the number of iclogs
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 15 Mar 2010 11:41:54 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1268620506-10799-10-git-send-email-david@xxxxxxxxxxxxx>
References: <1268620506-10799-1-git-send-email-david@xxxxxxxxxxxxx> <1268620506-10799-10-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
On Mon, Mar 15, 2010 at 01:35:06PM +1100, Dave Chinner wrote:
> +      * for LR headers - the space for data in an iclog is the size minus
> +      * the space used for the headers. If we use the iclog size, then we
> +      * undercalculate the number of headers required.
> +      *
> +      * Furthermore - the addition of op headers for split-recs might
> +      * increase the space required enough to require more log and op
> +      * headers, so take that into account too.
> +      *
> +      * IMPORTANT: This reservation makes the assumption that if this
> +      * transaction is the first in an iclog and hence has the LR headers
> +      * accounted to it, then the remaining space in the iclog is
> +      * exclusively for this transaction.  i.e. if the transaction is larger
> +      * than the iclog, it will be the only thing in that iclog.
> +      * Fundamentally, this means we must pass the entire log vector to
> +      * xlog_write to guarantee this.
> +      */
> +     iclog_space = log->l_iclog_size - log->l_iclog_hsize;
> +     num_headers = (unit_bytes + iclog_space - 1) / iclog_space;
> +
> +     /* for split-recs - ophdrs added when data split over LRs */
> +     unit_bytes += sizeof(xlog_op_header_t) * num_headers;
> +
> +     /* add extra header reservations if we overrun */
> +     while (!num_headers ||
> +            ((unit_bytes + iclog_space - 1) / iclog_space) > num_headers) {
> +             unit_bytes += sizeof(xlog_op_header_t);
> +             num_headers++;
> +     }

Looks good, but why do you check for a zero num_headers here?  The only way
this could happen after the roundup above is if unit_bytes is zero, which
can't ever happen - one caller has it hardcoded to 1, and the the other
has a conditional for it beeing bigger than 0 around the call.

<Prev in Thread] Current Thread [Next in Thread>