[Top] [All Lists]

Re: [RFC PATCH 2/4] xfs: add xfs_create_tmpfile() for O_TMPFILE support

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [RFC PATCH 2/4] xfs: add xfs_create_tmpfile() for O_TMPFILE support
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 25 Nov 2013 21:59:50 -0800
Cc: Zhi Yong Wu <zwu.kernel@xxxxxxxxx>, Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131125213601.GH8803@dastard>
References: <1385379154-3802-1-git-send-email-zwu.kernel@xxxxxxxxx> <1385379154-3802-3-git-send-email-zwu.kernel@xxxxxxxxx> <20131125213601.GH8803@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Nov 26, 2013 at 08:36:01AM +1100, Dave Chinner wrote:
> Is XFS_PROJID_DEFAULT correct here? If we are getting a parent inode
> from ->tmpfile, then this should be handled the same way as for
> xfs_create.

It should.  And while we're at it that code from create and symlink
should be factored into:

static inline projid_t xfs_initial_projid(struct xfs_inode *dp)
         if (dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
                return xfs_get_projid(dp);
        return XFS_PROJID_DEFAULT;


> I don't think this is necessary here. The ENOSPC flushing in
> xfs_create() is done to ensure we have space for directory block
> creation, not so much for inode allocation. Hence it doesn't make a
> lot of sense to have this here....

Point.  I take back my earlier comment that it should be factored.

> I'm not sure that XFS_MOUNT_DIRSYNC shoul dbe checked here, as there
> is no directory operations to synchronise at all...

It probably shouldn't indeed.  Reminds me of my old patch to make this
a flag to xfs_trans_commit instead of all that boilerplate code..

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