xfs
[Top] [All Lists]

Re: [PATCH 13/25] xfs: factor dir2 block read operations

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 13/25] xfs: factor dir2 block read operations
From: Phil White <pwhite@xxxxxxx>
Date: Mon, 29 Oct 2012 20:23:09 -0700
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1351146854-19343-14-git-send-email-david@xxxxxxxxxxxxx>
References: <1351146854-19343-1-git-send-email-david@xxxxxxxxxxxxx> <1351146854-19343-14-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Oct 25, 2012 at 05:34:02PM +1100, Dave Chinner wrote:
> +static void
> +xfs_dir2_block_need_space(
> ...
> +     /*
> +      * If there are stale entries we'll use one for the leaf.
> +      */
> +     if (btp->stale) {
> +             if (be16_to_cpu(bf[0].length) >= len) {
> +                     /*
> +                      * The biggest entry enough to avoid compaction.
> +                      */
> +                     dup = (xfs_dir2_data_unused_t *)
> +                           ((char *)hdr + be16_to_cpu(bf[0].offset));
> +                     goto out;
> +             }
> +
> +             /*
> +              * Will need to compact to make this work.
> +              * Tag just before the first leaf entry.
> +              */
> +             *compact = 1;
> +             tagp = (__be16 *)blp - 1;
> +
> +             /* Data object just before the first leaf entry.  */
> +             dup = (xfs_dir2_data_unused_t *)((char *)hdr + 
> be16_to_cpu(*tagp));
> +
> +             /*
> +              * If it's not free then the data will go where the
> +              * leaf data starts now, if it works at all.
> +              */
> +             if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
> +                     if (be16_to_cpu(dup->length) + (be32_to_cpu(btp->stale) 
> - 1) *
> +                         (uint)sizeof(*blp) < len)
> +                             dup = NULL;
> +             } else if ((be32_to_cpu(btp->stale) - 1) * (uint)sizeof(*blp) < 
> len)
> +                     dup = NULL;
> +             else
> +                     dup = (xfs_dir2_data_unused_t *)blp;
> +             goto out;
> +     }
> +
> +     /*
> +      * no stale entries, so just use free space.
> +      * Tag just before the first leaf entry.
> +      */
> +     tagp = (__be16 *)blp - 1;

Shouldn't tagp just be set before this if statement rather than inside of it
and outside of it?  In both cases it's equated to blp-1 and the value of blp
doesn't change to the best of my knowledge, so it's confusing to have two
assignments.

-Phil

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