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

[Christoph Hellwig <hch@infradead.org>]

> -	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.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs