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
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
