On Thu, Apr 10, 2014 at 03:29:01AM -0700, Christoph Hellwig wrote:
> > - 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.
>
Ok.
> > + } 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.
>
It's just as unique as the others. ;) I wasn't intending to call out
this transaction as special in any way. Rather, I was just trying to
document why there is a separate transaction depending on the existence
of the name. I can drop the comment.
> > + 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.
>
Indeed. I debated whether it would be reasonable to make this function
slightly longer and more complex on its own. When I realized
xfs_create_tmpfile() was 90% duplicate, it seemed worth the tradeoff for
a 100+ line function.
Brian
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
