Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1605553 > unrolled thread
| Started by | Chao Yu <yuchao0@huawei.com> |
|---|---|
| First post | 2017-03-21 13:20 +0100 |
| Last post | 2017-03-22 07:50 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 1/2] f2fs: introduce __drop_inmem_page Chao Yu <yuchao0@huawei.com> - 2017-03-21 13:20 +0100
Re: [PATCH 1/2] f2fs: introduce __drop_inmem_page Jaegeuk Kim <jaegeuk@kernel.org> - 2017-03-22 03:50 +0100
Re: [PATCH 1/2] f2fs: introduce __drop_inmem_page Chao Yu <yuchao0@huawei.com> - 2017-03-22 07:50 +0100
| From | Chao Yu <yuchao0@huawei.com> |
|---|---|
| Date | 2017-03-21 13:20 +0100 |
| Subject | [PATCH 1/2] f2fs: introduce __drop_inmem_page |
| Message-ID | <tnqVA-3d4-9@gated-at.bofh.it> |
Introduce a separated inner help __drop_inmem_page in order to:
a. enhance readability,
b. use INMEM_INVALIDATE for better tracing,
c. avoid useless loop.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/data.c | 2 +-
fs/f2fs/f2fs.h | 1 +
fs/f2fs/segment.c | 47 ++++++++++++++++++++++++++++++++++-----------
include/trace/events/f2fs.h | 2 ++
4 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f4b3006e3ecb..2d23c1f3d0f6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1967,7 +1967,7 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
/* This is atomic written page, keep Private */
if (IS_ATOMIC_WRITTEN_PAGE(page))
- drop_inmem_page(inode, page);
+ return drop_inmem_page(inode, page);
set_page_private(page, 0);
ClearPagePrivate(page);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 44c6c72d5e7d..82ddf5649d68 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -720,6 +720,7 @@ enum page_type {
META_FLUSH,
INMEM, /* the below types are used by tracepoints only. */
INMEM_DROP,
+ INMEM_INVALIDATE,
INMEM_REVOKE,
IPU,
OPU,
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 3e8d08722151..0bf024f62330 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -192,7 +192,7 @@ void register_inmem_page(struct inode *inode, struct page *page)
trace_f2fs_register_inmem_page(page, INMEM);
}
-static int __revoke_inmem_pages(struct inode *inode, struct page *p,
+static int __revoke_inmem_pages(struct inode *inode,
struct list_head *head, bool drop, bool recover)
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
@@ -205,10 +205,7 @@ static int __revoke_inmem_pages(struct inode *inode, struct page *p,
if (drop)
trace_f2fs_commit_inmem_page(page, INMEM_DROP);
- if (!p)
- lock_page(page);
- else if (p != page)
- continue;
+ lock_page(page);
if (recover) {
struct dnode_of_data dn;
@@ -232,7 +229,7 @@ static int __revoke_inmem_pages(struct inode *inode, struct page *p,
ClearPageUptodate(page);
set_page_private(page, 0);
ClearPagePrivate(page);
- f2fs_put_page(page, p ? 0 : 1);
+ f2fs_put_page(page, 1);
list_del(&cur->list);
kmem_cache_free(inmem_entry_slab, cur);
@@ -246,19 +243,47 @@ void drop_inmem_pages(struct inode *inode)
struct f2fs_inode_info *fi = F2FS_I(inode);
mutex_lock(&fi->inmem_lock);
- __revoke_inmem_pages(inode, NULL, &fi->inmem_pages, true, false);
+ __revoke_inmem_pages(inode, &fi->inmem_pages, true, false);
mutex_unlock(&fi->inmem_lock);
clear_inode_flag(inode, FI_ATOMIC_FILE);
stat_dec_atomic_write(inode);
}
+void __drop_inmem_page(struct inode *inode, struct page *page)
+{
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct list_head *head = &F2FS_I(inode)->inmem_pages;
+ struct inmem_pages *cur;
+
+ f2fs_bug_on(sbi, !IS_ATOMIC_WRITTEN_PAGE(page));
+
+ list_for_each_entry(cur, head, list) {
+ if (cur->page == page)
+ goto drop;
+ }
+
+ f2fs_bug_on(sbi, 1);
+ return;
+drop:
+ trace_f2fs_commit_inmem_page(page, INMEM_INVALIDATE);
+
+ ClearPageUptodate(page);
+ set_page_private(page, 0);
+ ClearPagePrivate(page);
+ f2fs_put_page(page, 0);
+
+ list_del(&cur->list);
+ kmem_cache_free(inmem_entry_slab, cur);
+ dec_page_count(sbi, F2FS_INMEM_PAGES);
+}
+
void drop_inmem_page(struct inode *inode, struct page *page)
{
struct f2fs_inode_info *fi = F2FS_I(inode);
mutex_lock(&fi->inmem_lock);
- __revoke_inmem_pages(inode, page, &fi->inmem_pages, true, false);
+ __drop_inmem_page(inode, page);
mutex_unlock(&fi->inmem_lock);
}
@@ -312,7 +337,7 @@ static int __commit_inmem_pages(struct inode *inode,
DATA, WRITE);
if (!err)
- __revoke_inmem_pages(inode, NULL, revoke_list, false, false);
+ __revoke_inmem_pages(inode, revoke_list, false, false);
return err;
}
@@ -342,13 +367,13 @@ int commit_inmem_pages(struct inode *inode)
* recovery or rewrite & commit last transaction. For other
* error number, revoking was done by filesystem itself.
*/
- ret = __revoke_inmem_pages(inode, NULL, &revoke_list,
+ ret = __revoke_inmem_pages(inode, &revoke_list,
false, true);
if (ret)
err = ret;
/* drop all uncommitted pages */
- __revoke_inmem_pages(inode, NULL, &fi->inmem_pages,
+ __revoke_inmem_pages(inode, &fi->inmem_pages,
true, false);
}
mutex_unlock(&fi->inmem_lock);
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 16fcd192e257..1714c75822de 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -15,6 +15,7 @@ TRACE_DEFINE_ENUM(META);
TRACE_DEFINE_ENUM(META_FLUSH);
TRACE_DEFINE_ENUM(INMEM);
TRACE_DEFINE_ENUM(INMEM_DROP);
+TRACE_DEFINE_ENUM(INMEM_INVALIDATE);
TRACE_DEFINE_ENUM(INMEM_REVOKE);
TRACE_DEFINE_ENUM(IPU);
TRACE_DEFINE_ENUM(OPU);
@@ -52,6 +53,7 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
{ META_FLUSH, "META_FLUSH" }, \
{ INMEM, "INMEM" }, \
{ INMEM_DROP, "INMEM_DROP" }, \
+ { INMEM_INVALIDATE, "INMEM_INVALIDATE" }, \
{ INMEM_REVOKE, "INMEM_REVOKE" }, \
{ IPU, "IN-PLACE" }, \
{ OPU, "OUT-OF-PLACE" })
--
2.8.2.295.g3f1c1d0
[toc] | [next] | [standalone]
| From | Jaegeuk Kim <jaegeuk@kernel.org> |
|---|---|
| Date | 2017-03-22 03:50 +0100 |
| Message-ID | <tnEvv-3Xg-5@gated-at.bofh.it> |
| In reply to | #1605553 |
Hi Chao,
Please let me make one patch by integrating into my original patch like this.
I slightly changed drop_inmem_page() as well.
Thanks,
From 8c242db9b8c01b252290e23827163787f07e01d1 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri, 17 Mar 2017 09:55:52 +0800
Subject: [PATCH] f2fs: fix stale ATOMIC_WRITTEN_PAGE private pointer
When I forced to enable atomic operations intentionally, I could hit the below
panic, since we didn't clear page->private in f2fs_invalidate_page called by
file truncation.
The panic occurs due to NULL mapping having page->private.
BUG: unable to handle kernel paging request at ffffffffffffffff
IP: drop_buffers+0x38/0xe0
PGD 5d00c067
PUD 5d00e067
PMD 0
CPU: 3 PID: 1648 Comm: fsstress Tainted: G D OE 4.10.0+ #5
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
task: ffff9151952863c0 task.stack: ffffaaec40db4000
RIP: 0010:drop_buffers+0x38/0xe0
RSP: 0018:ffffaaec40db74c8 EFLAGS: 00010292
Call Trace:
? page_referenced+0x8b/0x170
try_to_free_buffers+0xc5/0xe0
try_to_release_page+0x49/0x50
shrink_page_list+0x8bc/0x9f0
shrink_inactive_list+0x1dd/0x500
? shrink_active_list+0x2c0/0x430
shrink_node_memcg+0x5eb/0x7c0
shrink_node+0xe1/0x320
do_try_to_free_pages+0xef/0x2e0
try_to_free_pages+0xe9/0x190
__alloc_pages_slowpath+0x390/0xe70
__alloc_pages_nodemask+0x291/0x2b0
alloc_pages_current+0x95/0x140
__page_cache_alloc+0xc4/0xe0
pagecache_get_page+0xab/0x2a0
grab_cache_page_write_begin+0x20/0x40
get_read_data_page+0x2e6/0x4c0 [f2fs]
? f2fs_mark_inode_dirty_sync+0x16/0x30 [f2fs]
? truncate_data_blocks_range+0x238/0x2b0 [f2fs]
get_lock_data_page+0x30/0x190 [f2fs]
__exchange_data_block+0xaaf/0xf40 [f2fs]
f2fs_fallocate+0x418/0xd00 [f2fs]
vfs_fallocate+0x157/0x220
SyS_fallocate+0x48/0x80
Signed-off-by: Yunlei He <heyunlei@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
[Chao Yu: use INMEM_INVALIDATE for better tracing]
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/data.c | 2 +-
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/segment.c | 30 ++++++++++++++++++++++++++++++
include/trace/events/f2fs.h | 2 ++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1602b4bccae6..e341d446205a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1951,7 +1951,7 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
/* This is atomic written page, keep Private */
if (IS_ATOMIC_WRITTEN_PAGE(page))
- return;
+ return drop_inmem_page(inode, page);
set_page_private(page, 0);
ClearPagePrivate(page);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0a6e115562f6..264c219f41a5 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -722,6 +722,7 @@ enum page_type {
META_FLUSH,
INMEM, /* the below types are used by tracepoints only. */
INMEM_DROP,
+ INMEM_INVALIDATE,
INMEM_REVOKE,
IPU,
OPU,
@@ -2184,6 +2185,7 @@ void destroy_node_manager_caches(void);
*/
void register_inmem_page(struct inode *inode, struct page *page);
void drop_inmem_pages(struct inode *inode);
+void drop_inmem_page(struct inode *inode, struct page *page);
int commit_inmem_pages(struct inode *inode);
void f2fs_balance_fs(struct f2fs_sb_info *sbi, bool need);
void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 4d7bf84dc393..cb6d9ed634a3 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -250,6 +250,36 @@ void drop_inmem_pages(struct inode *inode)
stat_dec_atomic_write(inode);
}
+void drop_inmem_page(struct inode *inode, struct page *page)
+{
+ struct f2fs_inode_info *fi = F2FS_I(inode);
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct list_head *head = &fi->inmem_pages;
+ struct inmem_pages *cur = NULL;
+
+ f2fs_bug_on(sbi, !IS_ATOMIC_WRITTEN_PAGE(page));
+
+ mutex_lock(&fi->inmem_lock);
+ list_for_each_entry(cur, head, list) {
+ if (cur->page == page)
+ break;
+ }
+
+ f2fs_bug_on(sbi, !cur || cur->page != page);
+ list_del(&cur->list);
+ mutex_unlock(&fi->inmem_lock);
+
+ dec_page_count(sbi, F2FS_INMEM_PAGES);
+ kmem_cache_free(inmem_entry_slab, cur);
+
+ ClearPageUptodate(page);
+ set_page_private(page, 0);
+ ClearPagePrivate(page);
+ f2fs_put_page(page, 0);
+
+ trace_f2fs_commit_inmem_page(page, INMEM_INVALIDATE);
+}
+
static int __commit_inmem_pages(struct inode *inode,
struct list_head *revoke_list)
{
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index c80fcad0a6c9..96139021b6bc 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -15,6 +15,7 @@ TRACE_DEFINE_ENUM(META);
TRACE_DEFINE_ENUM(META_FLUSH);
TRACE_DEFINE_ENUM(INMEM);
TRACE_DEFINE_ENUM(INMEM_DROP);
+TRACE_DEFINE_ENUM(INMEM_INVALIDATE);
TRACE_DEFINE_ENUM(IPU);
TRACE_DEFINE_ENUM(OPU);
TRACE_DEFINE_ENUM(CURSEG_HOT_DATA);
@@ -51,6 +52,7 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
{ META_FLUSH, "META_FLUSH" }, \
{ INMEM, "INMEM" }, \
{ INMEM_DROP, "INMEM_DROP" }, \
+ { INMEM_INVALIDATE, "INMEM_INVALIDATE" }, \
{ INMEM_REVOKE, "INMEM_REVOKE" }, \
{ IPU, "IN-PLACE" }, \
{ OPU, "OUT-OF-PLACE" })
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Chao Yu <yuchao0@huawei.com> |
|---|---|
| Date | 2017-03-22 07:50 +0100 |
| Message-ID | <tnIfM-6Fs-17@gated-at.bofh.it> |
| In reply to | #1606167 |
Hi Jaegeuk,
On 2017/3/22 10:41, Jaegeuk Kim wrote:
> Hi Chao,
>
> Please let me make one patch by integrating into my original patch like this.
> I slightly changed drop_inmem_page() as well.
The merged patch looks good to me. ;)
Thanks,
>
> Thanks,
>
>>From 8c242db9b8c01b252290e23827163787f07e01d1 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Fri, 17 Mar 2017 09:55:52 +0800
> Subject: [PATCH] f2fs: fix stale ATOMIC_WRITTEN_PAGE private pointer
>
> When I forced to enable atomic operations intentionally, I could hit the below
> panic, since we didn't clear page->private in f2fs_invalidate_page called by
> file truncation.
>
> The panic occurs due to NULL mapping having page->private.
>
> BUG: unable to handle kernel paging request at ffffffffffffffff
> IP: drop_buffers+0x38/0xe0
> PGD 5d00c067
> PUD 5d00e067
> PMD 0
> CPU: 3 PID: 1648 Comm: fsstress Tainted: G D OE 4.10.0+ #5
> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> task: ffff9151952863c0 task.stack: ffffaaec40db4000
> RIP: 0010:drop_buffers+0x38/0xe0
> RSP: 0018:ffffaaec40db74c8 EFLAGS: 00010292
> Call Trace:
> ? page_referenced+0x8b/0x170
> try_to_free_buffers+0xc5/0xe0
> try_to_release_page+0x49/0x50
> shrink_page_list+0x8bc/0x9f0
> shrink_inactive_list+0x1dd/0x500
> ? shrink_active_list+0x2c0/0x430
> shrink_node_memcg+0x5eb/0x7c0
> shrink_node+0xe1/0x320
> do_try_to_free_pages+0xef/0x2e0
> try_to_free_pages+0xe9/0x190
> __alloc_pages_slowpath+0x390/0xe70
> __alloc_pages_nodemask+0x291/0x2b0
> alloc_pages_current+0x95/0x140
> __page_cache_alloc+0xc4/0xe0
> pagecache_get_page+0xab/0x2a0
> grab_cache_page_write_begin+0x20/0x40
> get_read_data_page+0x2e6/0x4c0 [f2fs]
> ? f2fs_mark_inode_dirty_sync+0x16/0x30 [f2fs]
> ? truncate_data_blocks_range+0x238/0x2b0 [f2fs]
> get_lock_data_page+0x30/0x190 [f2fs]
> __exchange_data_block+0xaaf/0xf40 [f2fs]
> f2fs_fallocate+0x418/0xd00 [f2fs]
> vfs_fallocate+0x157/0x220
> SyS_fallocate+0x48/0x80
>
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> [Chao Yu: use INMEM_INVALIDATE for better tracing]
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/data.c | 2 +-
> fs/f2fs/f2fs.h | 2 ++
> fs/f2fs/segment.c | 30 ++++++++++++++++++++++++++++++
> include/trace/events/f2fs.h | 2 ++
> 4 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1602b4bccae6..e341d446205a 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1951,7 +1951,7 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>
> /* This is atomic written page, keep Private */
> if (IS_ATOMIC_WRITTEN_PAGE(page))
> - return;
> + return drop_inmem_page(inode, page);
>
> set_page_private(page, 0);
> ClearPagePrivate(page);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 0a6e115562f6..264c219f41a5 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -722,6 +722,7 @@ enum page_type {
> META_FLUSH,
> INMEM, /* the below types are used by tracepoints only. */
> INMEM_DROP,
> + INMEM_INVALIDATE,
> INMEM_REVOKE,
> IPU,
> OPU,
> @@ -2184,6 +2185,7 @@ void destroy_node_manager_caches(void);
> */
> void register_inmem_page(struct inode *inode, struct page *page);
> void drop_inmem_pages(struct inode *inode);
> +void drop_inmem_page(struct inode *inode, struct page *page);
> int commit_inmem_pages(struct inode *inode);
> void f2fs_balance_fs(struct f2fs_sb_info *sbi, bool need);
> void f2fs_balance_fs_bg(struct f2fs_sb_info *sbi);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 4d7bf84dc393..cb6d9ed634a3 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -250,6 +250,36 @@ void drop_inmem_pages(struct inode *inode)
> stat_dec_atomic_write(inode);
> }
>
> +void drop_inmem_page(struct inode *inode, struct page *page)
> +{
> + struct f2fs_inode_info *fi = F2FS_I(inode);
> + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> + struct list_head *head = &fi->inmem_pages;
> + struct inmem_pages *cur = NULL;
> +
> + f2fs_bug_on(sbi, !IS_ATOMIC_WRITTEN_PAGE(page));
> +
> + mutex_lock(&fi->inmem_lock);
> + list_for_each_entry(cur, head, list) {
> + if (cur->page == page)
> + break;
> + }
> +
> + f2fs_bug_on(sbi, !cur || cur->page != page);
> + list_del(&cur->list);
> + mutex_unlock(&fi->inmem_lock);
> +
> + dec_page_count(sbi, F2FS_INMEM_PAGES);
> + kmem_cache_free(inmem_entry_slab, cur);
> +
> + ClearPageUptodate(page);
> + set_page_private(page, 0);
> + ClearPagePrivate(page);
> + f2fs_put_page(page, 0);
> +
> + trace_f2fs_commit_inmem_page(page, INMEM_INVALIDATE);
> +}
> +
> static int __commit_inmem_pages(struct inode *inode,
> struct list_head *revoke_list)
> {
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c80fcad0a6c9..96139021b6bc 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -15,6 +15,7 @@ TRACE_DEFINE_ENUM(META);
> TRACE_DEFINE_ENUM(META_FLUSH);
> TRACE_DEFINE_ENUM(INMEM);
> TRACE_DEFINE_ENUM(INMEM_DROP);
> +TRACE_DEFINE_ENUM(INMEM_INVALIDATE);
> TRACE_DEFINE_ENUM(IPU);
> TRACE_DEFINE_ENUM(OPU);
> TRACE_DEFINE_ENUM(CURSEG_HOT_DATA);
> @@ -51,6 +52,7 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
> { META_FLUSH, "META_FLUSH" }, \
> { INMEM, "INMEM" }, \
> { INMEM_DROP, "INMEM_DROP" }, \
> + { INMEM_INVALIDATE, "INMEM_INVALIDATE" }, \
> { INMEM_REVOKE, "INMEM_REVOKE" }, \
> { IPU, "IN-PLACE" }, \
> { OPU, "OUT-OF-PLACE" })
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web