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
|