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 18:47:26 +0800
Cc: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>, xfstests <xfs@xxxxxxxxxxx>
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=m1Asi/ILygfnujLpS+HnwiaFfFnl5knikKJgzjSw1nc=; b=OhSxLRggSh0UrIl94tL91EaCPKFaUJm7GER8isVpPfssXIo71NxFt5Z+4HKShxTTNd FxrIfwjxKz1tQOtRrfcH8IkIUbnJLli/KGlp1fzjPGWo99BAiuP8CkrSKlvIQA3MjwBj U3+aebEi5mkHPGrhEH/MmKWtxxZR5QeiCJa5GYmUEpL/yAu4zv62TGoa32mCFOawiBRs 2zf1st50F1DrpaUdFDCJ9zJ/qkjpglAnNne+WqpR3oAA7NJWzfmSn3Yj+6H95qu6nOm+ wFHhbOka40T3LBUXKzrraWBPUhA/u57Hv1uhfhrNO5VUsmv2dX0cvSGnY3hAIzy5DFvk PNCw==
In-reply-to: <20131128103930.GB26927@xxxxxxxxxxxxx>
References: <1385379154-3802-1-git-send-email-zwu.kernel@xxxxxxxxx> <1385379154-3802-5-git-send-email-zwu.kernel@xxxxxxxxx> <20131125135149.GD30189@xxxxxxxxxxxxx> <CAEH94LiiRgQGzf+YGy23SJDhHJ_1grC3kniLqXDdUQFJ09McTA@xxxxxxxxxxxxxx> <20131128103930.GB26927@xxxxxxxxxxxxx>
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

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