xfs
[Top] [All Lists]

Re: [RFC PATCH 4/4] xfs: allow linkat() on O_TMPFILE files

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [RFC PATCH 4/4] xfs: allow linkat() on O_TMPFILE files
From: Zhi Yong Wu <zwu.kernel@xxxxxxxxx>
Date: Thu, 28 Nov 2013 10:37:29 +0800
Cc: xfstests <xfs@xxxxxxxxxxx>, Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=fN24UeGCTzu3J8frUHaGVlkdHR2hdhAJBERfJCRRURs=; b=0pzsTQPI4OjqaPqQRv/Z0F5TQI1qhf2K7JF/2KBBsW7fluqsCjo2UmRTj1rd4ezrp3 XqUQ2BKLQuT71kP8YTxenhSJHo0huUUSr+hbThipnBGwMW1sLkeT3QJ/8e2BNAQ+hkDX BWi4TP/oLPbVORDXIRCuLbeGofzvszJZB+uf90pgqW8mqOpFt2Wz/1UXJ6V0vYwuCINa fk7IDDj32J20r1BpuOYh2tepNJ7uvLd4dLQjnJ3QHC/InnbTa4o142UKMD0HcHxsHy4t F9MM4uXkjrmpE+X1TscSun6jqYvmEIHCyObMINX6sMGO2pckTO9R6qNtX9MbCsjJlIgd sH7A==
In-reply-to: <20131125135149.GD30189@xxxxxxxxxxxxx>
References: <1385379154-3802-1-git-send-email-zwu.kernel@xxxxxxxxx> <1385379154-3802-5-git-send-email-zwu.kernel@xxxxxxxxx> <20131125135149.GD30189@xxxxxxxxxxxxx>
On Mon, Nov 25, 2013 at 9:51 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>> -     ASSERT(ip->i_d.di_nlink > 0);
>> +     if ((VFS_I(ip)->i_nlink == 0) &&
>> +         !(VFS_I(ip)->i_state & I_LINKABLE))
>> +             ASSERT(ip->i_d.di_nlink > 0);
>
>         ASSERT(ip->i_d.di_nlink > 0 || (VFS_I(ip)->i_state & I_LINKABLE));
This is wrong, and it should be
          ASSERT(ip->i_d.di_nlink > 0 || !(VFS_I(ip)->i_state & I_LINKABLE));

>
>> @@ -1498,7 +1503,14 @@ xfs_link(
>>       tp = xfs_trans_alloc(mp, XFS_TRANS_LINK);
>>       cancel_flags = XFS_TRANS_RELEASE_LOG_RES;
>>       resblks = XFS_LINK_SPACE_RES(mp, target_name->len);
>> -     error = xfs_trans_reserve(tp, &M_RES(mp)->tr_link, resblks, 0);
>> +
>> +     if ((VFS_I(sip)->i_nlink == 0) &&
>> +         (VFS_I(sip)->i_state & I_LINKABLE))
>> +             tres = &M_RES(mp)->tr_link_tmpfile;
>> +     else
>> +             tres = &M_RES(mp)->tr_link;
>
> Just check i_nlink, and for consistency it might make sense to just use
> the xfs_inode one.  The VFS already made sure we don't inodes with
but struct xfs_inode has no stuff similar to i_nlink....

> I_LINKABLE and a zero link count.
No, pls see the chunk of code:
int vfs_link(struct dentry *old_dentry, struct inode *dir, struct
dentry *new_dentry)
{
...
    /* Make sure we don't allow creating hardlink to an unlinked file */
    if (inode->i_nlink == 0 && !(inode->i_state & I_LINKABLE))
        error =  -ENOENT;
    else if (max_links && inode->i_nlink >= max_links)
        error = -EMLINK;
    else
        error = dir->i_op->link(old_dentry, dir, new_dentry);

    if (!error && (inode->i_state & I_LINKABLE)) {
        spin_lock(&inode->i_lock);
        inode->i_state &= ~I_LINKABLE;
        spin_unlock(&inode->i_lock);
    }
...
}

When xfs_link() is called, I_LINKABLE flag isn't cleared.

>
>> +     if ((VFS_I(sip)->i_nlink == 0) &&
>> +         (VFS_I(sip)->i_state & I_LINKABLE)) {
>
> Same here.
>



-- 
Regards,

Zhi Yong Wu

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