* [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
@ 2016-08-25 2:34 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-08-25 2:34 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim
When a file is closed, let's deactivate inode page to mitigate further memory
pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/file.c | 4 ++++
fs/f2fs/super.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e460211..5f9a6dc 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
filemap_fdatawrite(inode->i_mapping);
clear_inode_flag(inode, FI_DROP_CACHE);
}
+
+ /* deactivate written inode page */
+ invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
+ inode->i_ino, inode->i_ino);
return 0;
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7f863a6..f84696d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
sb_end_intwrite(inode->i_sb);
fscrypt_put_encryption_info(inode, NULL);
+ invalidate_mapping_pages(inode->i_mapping, 0, -1);
spin_lock(&inode->i_lock);
atomic_dec(&inode->i_count);
}
--
2.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
@ 2016-08-25 2:34 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-08-25 2:34 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim
When a file is closed, let's deactivate inode page to mitigate further memory
pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/file.c | 4 ++++
fs/f2fs/super.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e460211..5f9a6dc 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
filemap_fdatawrite(inode->i_mapping);
clear_inode_flag(inode, FI_DROP_CACHE);
}
+
+ /* deactivate written inode page */
+ invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
+ inode->i_ino, inode->i_ino);
return 0;
}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 7f863a6..f84696d 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
sb_end_intwrite(inode->i_sb);
fscrypt_put_encryption_info(inode, NULL);
+ invalidate_mapping_pages(inode->i_mapping, 0, -1);
spin_lock(&inode->i_lock);
atomic_dec(&inode->i_count);
}
--
2.8.3
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
2016-08-25 2:34 ` Jaegeuk Kim
(?)
@ 2016-08-25 11:59 ` Chao Yu
2016-08-25 17:00 ` Jaegeuk Kim
-1 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2016-08-25 11:59 UTC (permalink / raw)
To: linux-f2fs-devel
Hi Jaegeuk,
On 2016/8/25 10:34, Jaegeuk Kim wrote:
> When a file is closed, let's deactivate inode page to mitigate further memory
> pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/file.c | 4 ++++
> fs/f2fs/super.c | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index e460211..5f9a6dc 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
> filemap_fdatawrite(inode->i_mapping);
> clear_inode_flag(inode, FI_DROP_CACHE);
> }
> +
> + /* deactivate written inode page */
> + invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
> + inode->i_ino, inode->i_ino);
Seems if the file was opened by two processes, one process closes this file,
another one will loss page cache of inode. I'm not sure whether this predication
is friendly to user.
> return 0;
> }
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 7f863a6..f84696d 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
> sb_end_intwrite(inode->i_sb);
>
> fscrypt_put_encryption_info(inode, NULL);
> + invalidate_mapping_pages(inode->i_mapping, 0, -1);
Hmm, shouldn't we truncate page cache before f2fs_truncate() like we do in
f2fs_evict?
Thanks,
> spin_lock(&inode->i_lock);
> atomic_dec(&inode->i_count);
> }
>
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
2016-08-25 11:59 ` Chao Yu
@ 2016-08-25 17:00 ` Jaegeuk Kim
2016-08-25 17:04 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-08-25 17:00 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel
On Thu, Aug 25, 2016 at 07:59:45PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> On 2016/8/25 10:34, Jaegeuk Kim wrote:
> > When a file is closed, let's deactivate inode page to mitigate further memory
> > pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > fs/f2fs/file.c | 4 ++++
> > fs/f2fs/super.c | 1 +
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index e460211..5f9a6dc 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
> > filemap_fdatawrite(inode->i_mapping);
> > clear_inode_flag(inode, FI_DROP_CACHE);
> > }
> > +
> > + /* deactivate written inode page */
> > + invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
> > + inode->i_ino, inode->i_ino);
>
> Seems if the file was opened by two processes, one process closes this file,
> another one will loss page cache of inode. I'm not sure whether this predication
> is friendly to user.
No, this only calls when it's the last writer. BTW, I need to add reader case
as well. :)
>
> > return 0;
> > }
> >
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 7f863a6..f84696d 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
> > sb_end_intwrite(inode->i_sb);
> >
> > fscrypt_put_encryption_info(inode, NULL);
> > + invalidate_mapping_pages(inode->i_mapping, 0, -1);
>
> Hmm, shouldn't we truncate page cache before f2fs_truncate() like we do in
> f2fs_evict?
Then, it needs to be done under i_lock, and IMO, we don't need to wait for
writeback here.
Thanks,
>
> Thanks,
>
> > spin_lock(&inode->i_lock);
> > atomic_dec(&inode->i_count);
> > }
> >
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
2016-08-25 17:00 ` Jaegeuk Kim
@ 2016-08-25 17:04 ` Jaegeuk Kim
2016-08-26 1:14 ` Chao Yu
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-08-25 17:04 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel
On Thu, Aug 25, 2016 at 10:00:37AM -0700, Jaegeuk Kim wrote:
> On Thu, Aug 25, 2016 at 07:59:45PM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> >
> > On 2016/8/25 10:34, Jaegeuk Kim wrote:
> > > When a file is closed, let's deactivate inode page to mitigate further memory
> > > pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
> > >
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > > fs/f2fs/file.c | 4 ++++
> > > fs/f2fs/super.c | 1 +
> > > 2 files changed, 5 insertions(+)
> > >
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index e460211..5f9a6dc 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
> > > filemap_fdatawrite(inode->i_mapping);
> > > clear_inode_flag(inode, FI_DROP_CACHE);
> > > }
> > > +
> > > + /* deactivate written inode page */
> > > + invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
> > > + inode->i_ino, inode->i_ino);
> >
> > Seems if the file was opened by two processes, one process closes this file,
> > another one will loss page cache of inode. I'm not sure whether this predication
> > is friendly to user.
>
> No, this only calls when it's the last writer. BTW, I need to add reader case
> as well. :)
if (!(filp->f_mode & FMODE_WRITE) ||
atomic_read(&inode->i_writecount) != 1)
return 0;
So, it drops its inode page only when the last writer is closed.
Thanks,
>
> >
> > > return 0;
> > > }
> > >
> > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > index 7f863a6..f84696d 100644
> > > --- a/fs/f2fs/super.c
> > > +++ b/fs/f2fs/super.c
> > > @@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
> > > sb_end_intwrite(inode->i_sb);
> > >
> > > fscrypt_put_encryption_info(inode, NULL);
> > > + invalidate_mapping_pages(inode->i_mapping, 0, -1);
> >
> > Hmm, shouldn't we truncate page cache before f2fs_truncate() like we do in
> > f2fs_evict?
>
> Then, it needs to be done under i_lock, and IMO, we don't need to wait for
> writeback here.
>
> Thanks,
>
> >
> > Thanks,
> >
> > > spin_lock(&inode->i_lock);
> > > atomic_dec(&inode->i_count);
> > > }
> > >
> >
> >
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > 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
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] f2fs: invalidate inode and data pages if inode is no longer used
2016-08-25 17:04 ` Jaegeuk Kim
@ 2016-08-26 1:14 ` Chao Yu
0 siblings, 0 replies; 6+ messages in thread
From: Chao Yu @ 2016-08-26 1:14 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel
On 2016/8/26 1:04, Jaegeuk Kim wrote:
> On Thu, Aug 25, 2016 at 10:00:37AM -0700, Jaegeuk Kim wrote:
>> On Thu, Aug 25, 2016 at 07:59:45PM +0800, Chao Yu wrote:
>>> Hi Jaegeuk,
>>>
>>> On 2016/8/25 10:34, Jaegeuk Kim wrote:
>>>> When a file is closed, let's deactivate inode page to mitigate further memory
>>>> pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
>>>>
>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>> ---
>>>> fs/f2fs/file.c | 4 ++++
>>>> fs/f2fs/super.c | 1 +
>>>> 2 files changed, 5 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index e460211..5f9a6dc 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
>>>> filemap_fdatawrite(inode->i_mapping);
>>>> clear_inode_flag(inode, FI_DROP_CACHE);
>>>> }
>>>> +
>>>> + /* deactivate written inode page */
>>>> + invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
>>>> + inode->i_ino, inode->i_ino);
>>>
>>> Seems if the file was opened by two processes, one process closes this file,
>>> another one will loss page cache of inode. I'm not sure whether this predication
>>> is friendly to user.
>>
>> No, this only calls when it's the last writer. BTW, I need to add reader case
>> as well. :)
Oh, right.
>
> if (!(filp->f_mode & FMODE_WRITE) ||
> atomic_read(&inode->i_writecount) != 1)
> return 0;
>
> So, it drops its inode page only when the last writer is closed.
Reading data of inode will trigger atime updating, should we consider to keep
inode page for read?
Thanks,
>
> Thanks,
>
>>
>>>
>>>> return 0;
>>>> }
>>>>
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index 7f863a6..f84696d 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
>>>> sb_end_intwrite(inode->i_sb);
>>>>
>>>> fscrypt_put_encryption_info(inode, NULL);
>>>> + invalidate_mapping_pages(inode->i_mapping, 0, -1);
>>>
>>> Hmm, shouldn't we truncate page cache before f2fs_truncate() like we do in
>>> f2fs_evict?
>>
>> Then, it needs to be done under i_lock, and IMO, we don't need to wait for
>> writeback here.
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>> spin_lock(&inode->i_lock);
>>>> atomic_dec(&inode->i_count);
>>>> }
>>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> _______________________________________________
>>> 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
>
> .
>
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-26 2:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-25 2:34 [PATCH] f2fs: invalidate inode and data pages if inode is no longer used Jaegeuk Kim
2016-08-25 2:34 ` Jaegeuk Kim
2016-08-25 11:59 ` Chao Yu
2016-08-25 17:00 ` Jaegeuk Kim
2016-08-25 17:04 ` Jaegeuk Kim
2016-08-26 1:14 ` Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.