Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 28 Sep 2019 03:01:10 +0800
From:   Gao Xiang <hsiangkao@aol.com>
To:     Jaegeuk Kim <jaegeuk@kernel.org>
Cc:     Chao Yu <yuchao0@huawei.com>,
        linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, chao@kernel.org
Subject: Re: [PATCH] f2fs: fix comment of f2fs_evict_inode
Message-ID: <20190927190106.GA27964@hsiangkao-HP-ZHAN-66-Pro-G1>
References: <20190925093050.118921-1-yuchao0@huawei.com>
 <20190927183150.GA54001@jaegeuk-macbookpro.roam.corp.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190927183150.GA54001@jaegeuk-macbookpro.roam.corp.google.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jaegeuk,

On Fri, Sep 27, 2019 at 11:31:50AM -0700, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On 09/25, Chao Yu wrote:
> > evict() should be called once i_count is zero, rather than i_nlinke
> > is zero.
> > 
> > Reported-by: Gao Xiang <gaoxiang25@huawei.com>
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> >  fs/f2fs/inode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> > index db4fec30c30d..8262f4a483d3 100644
> > --- a/fs/f2fs/inode.c
> > +++ b/fs/f2fs/inode.c
> > @@ -632,7 +632,7 @@ int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
> >  }
> >  
> >  /*
> > - * Called at the last iput() if i_nlink is zero
> 
> I don't think this comment is wrong. You may be able to add on top of this.

Actually I don't really care what this line means, but someone really
told me that .evict_inode() is called on inode is finally removed
because he saw this line.

In practice, I have no idea what the above line (especially the word i_nlink
== 0) mainly emphasizes, just from some documentation (not even refer some code):

Documentation/filesystems/porting.rst
326 **mandatory**
327 
328 ->clear_inode() and ->delete_inode() are gone; ->evict_inode() should
329 be used instead.  It gets called whenever the inode is evicted, whether it has
330 remaining links or not. 

And it seems it's the same comment exists in ext2/ext4. But yes, it's up
to you. However, it misleaded someone and I had to explain more about this.

Thanks,
Gao Xiang

> 
> > + * Called at the last iput() if i_count is zero
> >   */
> >  void f2fs_evict_inode(struct inode *inode)
> >  {
> > -- 
> > 2.18.0.rc1