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

[Zhi Yong Wu <zwu.kernel@xxxxxxxxx>]

You are right for both following cases, and will fix them, thanks.

On Thu, Nov 28, 2013 at 6:39 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Thu, Nov 28, 2013 at 10:37:29AM +0800, Zhi Yong Wu wrote:
>> 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));
>
> Why we want to assrrt that either the link count is bigger than 0,
> or that the I_LINKABLE flag is set (for files created using O_TMPFILE)
>
>> >> +
>> >> +     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....
>
> ip->i_d.di_nlink is the equivalent.
>
>> > 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;
>
> This makes sure we never created a link if the count is zero unless
> the I_LINKABLE is set, so we'll never see a zero link count without
> I_LINKABLE.
>



-- 
Regards,

Zhi Yong Wu