From: Gao Xiang <gaoxiang25@huawei.com>
To: <linux-f2fs-devel@lists.sourceforge.net>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix comment of f2fs_evict_inode
Date: Sun, 29 Sep 2019 10:20:52 +0800 [thread overview]
Message-ID: <20190929021939.GA136917@architecture4> (raw)
In-Reply-To: <8c54adaf-163f-fcbe-7731-0c18b12410e0@huawei.com>
On Sun, Sep 29, 2019 at 08:53:05AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> On 2019/9/28 2:31, 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.
>
> It actually misleads the developer or user.
>
> How do you think of:
>
> "Called at the last iput() if i_count is zero, and will release all meta/data
> blocks allocated in the inode if i_nlink is zero"
(sigh... side note: I just took some time to check the original meaning
out of curiosity. AFAIK, the above word was added in Linux-2.1.45 [1]
due to ext2_delete_inode behavior, which is called when i_nlink == 0,
and .delete_inode was gone in 2010 (commit 72edc4d0873b merge ext2
delete_inode and clear_inode, switch to ->evict_inode()), it may be
helpful to understand the story so I write here for later folks reference.
And it's also good to just kill it. )
+
+/*
+ * Called at the last iput() if i_nlink is zero.
+ */
+void ext2_delete_inode (struct inode * inode)
+{
+ if (inode->i_ino == EXT2_ACL_IDX_INO ||
inode->i_ino == EXT2_ACL_DATA_INO)
return;
inode->u.ext2_i.i_dtime = CURRENT_TIME;
- inode->i_dirt = 1;
+ mark_inode_dirty(inode);
ext2_update_inode(inode, IS_SYNC(inode));
inode->i_size = 0;
if (inode->i_blocks)
@@ -248,7 +258,7 @@
if (IS_SYNC(inode) || inode->u.ext2_i.i_osync)
ext2_sync_inode (inode);
else
- inode->i_dirt = 1;
+ mark_inode_dirty(inode);
return result;
}
+void iput(struct inode *inode)
{
- struct inode * inode = get_empty_inode();
+ if (inode) {
+ struct super_operations *op = NULL;
- PIPE_BASE(*inode) = (char*)__get_free_page(GFP_USER);
- if (!(PIPE_BASE(*inode))) {
- iput(inode);
- return NULL;
+ if (inode->i_sb && inode->i_sb->s_op)
+ op = inode->i_sb->s_op;
+ if (op && op->put_inode)
+ op->put_inode(inode);
+
+ spin_lock(&inode_lock);
+ if (!--inode->i_count) {
+ if (!inode->i_nlink) {
+ list_del(&inode->i_hash);
+ INIT_LIST_HEAD(&inode->i_hash);
+ if (op && op->delete_inode) {
+ void (*delete)(struct inode *) = op->delete_inode;
+ spin_unlock(&inode_lock);
+ delete(inode);
+ spin_lock(&inode_lock);
+ }
+ }
+ if (list_empty(&inode->i_hash)) {
+ list_del(&inode->i_list);
+ list_add(&inode->i_list, &inode_unused);
+ }
+ }
+ spin_unlock(&inode_lock);
}
[1] https://www.kernel.org/pub/linux/kernel/v2.1/patch-2.1.45.xz
Thanks,
Gao Xiang
>
> Thanks,
>
> >
> >> + * Called at the last iput() if i_count is zero
> >> */
> >> void f2fs_evict_inode(struct inode *inode)
> >> {
> >> --
> >> 2.18.0.rc1
> > .
> >
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-09-29 2:22 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 9:30 [f2fs-dev] [PATCH] f2fs: fix comment of f2fs_evict_inode Chao Yu
2019-09-25 13:47 ` Gao Xiang
2019-09-26 1:04 ` Chao Yu
2019-09-27 18:31 ` Jaegeuk Kim
2019-09-27 19:01 ` Gao Xiang via Linux-f2fs-devel
2019-09-29 0:53 ` Chao Yu
2019-09-29 2:20 ` Gao Xiang [this message]
2019-09-29 2:52 ` Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190929021939.GA136917@architecture4 \
--to=gaoxiang25@huawei.com \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).