xfs
[Top] [All Lists]

Re: [PATCH v2 2/2] xfs: fold xfs_create_tmpfile() into xfs_create()

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v2 2/2] xfs: fold xfs_create_tmpfile() into xfs_create()
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 10 Apr 2014 03:29:01 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1397071311-28371-3-git-send-email-bfoster@xxxxxxxxxx>
References: <1397071311-28371-1-git-send-email-bfoster@xxxxxxxxxx> <1397071311-28371-3-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
> -     struct xfs_trans_res    tres;
> +     struct xfs_trans_res    *tres;
>       uint                    resblks;
>  
>       trace_xfs_create(dp, name);
> @@ -1181,14 +1181,21 @@ xfs_create(
>       if (is_dir) {
>               rdev = 0;
>               resblks = XFS_MKDIR_SPACE_RES(mp, name->len);
> -             tres.tr_logres = M_RES(mp)->tr_mkdir.tr_logres;
> -             tres.tr_logcount = XFS_MKDIR_LOG_COUNT;
> +             tres = &M_RES(mp)->tr_mkdir;

The (nice) reservation cleanup should be a patch of it's own. 

> +     } else {
> +             /*
> +              * If we don't have a name, we're in the ->tmpfile() path. We
> +              * have a unique transaction here since we modify the unlinked
> +              * list rather than create a directory entry.
> +              */

How is that transaction more "uniqueue" than the others?  Seems like
this comment generally doesn't add a whole lot of value.

> +     if (name) {
> +             xfs_ilock(dp, XFS_ILOCK_EXCL | XFS_ILOCK_PARENT);
> +             unlock_dp_on_error = true;
> +
> +             error = xfs_dir_canenter(tp, dp, name, resblks);
> +             if (error)
> +                     goto out_trans_cancel;
> +     }

So we get another special case in this function.  Can't say I like that
too much, on the other hand I don't really like the duplicate code
either.  So I'm not excited about this, but also not strongly against it.

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