| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e255db23-ef5a-244e-879a-bc813443042a@huawei.com>
Date: Tue, 1 Aug 2017 14:10:02 +0800
From: Yunlong Song <yunlong.song@...wei.com>
To: Jaegeuk Kim <jaegeuk@...nel.org>
CC: <chao@...nel.org>, <yuchao0@...wei.com>, <sylinux@....com>,
<miaoxie@...wei.com>, <bintian.wang@...wei.com>,
<linux-fsdevel@...r.kernel.org>,
<linux-f2fs-devel@...ts.sourceforge.net>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] f2fs: provide f2fs_balance_fs to __write_data_page for
dentry pages
Hi Jay,
The SBI_BLOCK_OPS can not cover all the case, once SBI_BLOCK_OPS is
set, all the normal writeback
(before the SBI_BLOCK_OPS is clear) of dentry pages which do not come
from write_checkpoint flow will
totally miss all the f2fs_balance_fs check.
On 2017/8/1 8:44, Jaegeuk Kim wrote:
> Hi Yunlong,
>
> On 07/29, Yunlong Song wrote:
>> f2fs_balance_fs of dentry pages is skipped in __write_data_page due to deadlock
>> of gc_mutex in write_checkpoint flow. This patch enables f2fs_balance_fs for
>> normal dentry page writeback to ensure there are always enough free segments.
> So, how about adding SBI_BLOCK_OPS
>
> /* For s_flag in struct f2fs_sb_info */
> enum {
> SBI_IS_DIRTY, /* dirty flag for checkpoint */
> SBI_IS_CLOSE, /* specify unmounting */
> SBI_NEED_FSCK, /* need fsck.f2fs to fix */
> SBI_POR_DOING, /* recovery is doing or not */
> SBI_NEED_SB_WRITE, /* need to recover superblock */
> SBI_NEED_CP, /* need to checkpoint */
> ===
> SBI_BLOCK_OPS, /* doing block_operations */
> ===
> };
>
> How aboud avoiding f2fs_balance_fs() in __write_data_page() by:
>
> if (!is_sbi_flag_set(sbi, SBI_BLOCK_OPS) || !S_ISDIR(inode->i_mode))
> f2fs_balance_fs();
>
> In block_operations(),
> set_sbi_flag(sbi, SBI_BLOCK_OPS);
> ...
> clear_sbi_flag(sbi, SBI_BLOCK_OPS);
>
> Thanks,
>
>> Reported-by: Chao Yu <yuchao0@...wei.com>
>> Signed-off-by: Yunlong Song <yunlong.song@...wei.com>
>> ---
>> fs/f2fs/checkpoint.c | 2 +-
>> fs/f2fs/data.c | 67 +++++++++++++++++++++++++++++++++++++++++++++-------
>> fs/f2fs/f2fs.h | 1 +
>> 3 files changed, 61 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 3c84a25..2882878 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -904,7 +904,7 @@ int sync_dirty_inodes(struct f2fs_sb_info *sbi, enum inode_type type)
>> if (inode) {
>> unsigned long cur_ino = inode->i_ino;
>>
>> - filemap_fdatawrite(inode->i_mapping);
>> + f2fs_filemap_fdatawrite(inode->i_mapping, is_dir);
>> iput(inode);
>> /* We need to give cpu to another writers. */
>> if (ino == cur_ino) {
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index aefc2a5..ed7efa5 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1475,7 +1475,7 @@ int do_write_data_page(struct f2fs_io_info *fio)
>> }
>>
>> static int __write_data_page(struct page *page, bool *submitted,
>> - struct writeback_control *wbc)
>> + struct writeback_control *wbc, bool do_balance)
>> {
>> struct inode *inode = page->mapping->host;
>> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> @@ -1578,7 +1578,7 @@ static int __write_data_page(struct page *page, bool *submitted,
>> }
>>
>> unlock_page(page);
>> - if (!S_ISDIR(inode->i_mode))
>> + if (do_balance)
>> f2fs_balance_fs(sbi, need_balance_fs);
>>
>> if (unlikely(f2fs_cp_error(sbi))) {
>> @@ -1602,7 +1602,7 @@ static int __write_data_page(struct page *page, bool *submitted,
>> static int f2fs_write_data_page(struct page *page,
>> struct writeback_control *wbc)
>> {
>> - return __write_data_page(page, NULL, wbc);
>> + return __write_data_page(page, NULL, wbc, true);
>> }
>>
>> /*
>> @@ -1611,7 +1611,7 @@ static int f2fs_write_data_page(struct page *page,
>> * warm/hot data page.
>> */
>> static int f2fs_write_cache_pages(struct address_space *mapping,
>> - struct writeback_control *wbc)
>> + struct writeback_control *wbc, bool do_balance)
>> {
>> int ret = 0;
>> int done = 0;
>> @@ -1701,7 +1701,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>> if (!clear_page_dirty_for_io(page))
>> goto continue_unlock;
>>
>> - ret = __write_data_page(page, &submitted, wbc);
>> + ret = __write_data_page(page, &submitted, wbc, do_balance);
>> if (unlikely(ret)) {
>> /*
>> * keep nr_to_write, since vfs uses this to
>> @@ -1756,8 +1756,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>> return ret;
>> }
>>
>> -static int f2fs_write_data_pages(struct address_space *mapping,
>> - struct writeback_control *wbc)
>> +static int _f2fs_write_data_pages(struct address_space *mapping,
>> + struct writeback_control *wbc, bool do_balance)
>> {
>> struct inode *inode = mapping->host;
>> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> @@ -1794,7 +1794,7 @@ static int f2fs_write_data_pages(struct address_space *mapping,
>> goto skip_write;
>>
>> blk_start_plug(&plug);
>> - ret = f2fs_write_cache_pages(mapping, wbc);
>> + ret = f2fs_write_cache_pages(mapping, wbc, do_balance);
>> blk_finish_plug(&plug);
>>
>> if (wbc->sync_mode == WB_SYNC_ALL)
>> @@ -1813,6 +1813,57 @@ static int f2fs_write_data_pages(struct address_space *mapping,
>> return 0;
>> }
>>
>> +static int f2fs_write_data_pages(struct address_space *mapping,
>> + struct writeback_control *wbc)
>> +{
>> + return _f2fs_write_data_pages(mapping, wbc, true);
>> +}
>> +
>> +/*
>> + * This function was copied from do_writepages from mm/page-writeback.c.
>> + * The major change is changing writepages to _f2fs_write_data_pages.
>> + */
>> +static int f2fs_do_writepages(struct address_space *mapping,
>> + struct writeback_control *wbc, bool is_dir)
>> +{
>> + int ret;
>> +
>> + if (wbc->nr_to_write <= 0)
>> + return 0;
>> + while (1) {
>> + ret = _f2fs_write_data_pages(mapping, wbc, !is_dir);
>> + if ((ret != -ENOMEM) || (wbc->sync_mode != WB_SYNC_ALL))
>> + break;
>> + cond_resched();
>> + congestion_wait(BLK_RW_ASYNC, HZ/50);
>> + }
>> + return ret;
>> +}
>> +
>> +/*
>> + * This function was copied from __filemap_fdatawrite_range from
>> + * mm/filemap.c. The major change is changing do_writepages to
>> + * f2fs_do_writepages.
>> + */
>> +int f2fs_filemap_fdatawrite(struct address_space *mapping, bool is_dir)
>> +{
>> + int ret;
>> + struct writeback_control wbc = {
>> + .sync_mode = WB_SYNC_ALL,
>> + .nr_to_write = LONG_MAX,
>> + .range_start = 0,
>> + .range_end = LLONG_MAX,
>> + };
>> +
>> + if (!mapping_cap_writeback_dirty(mapping))
>> + return 0;
>> +
>> + wbc_attach_fdatawrite_inode(&wbc, mapping->host);
>> + ret = f2fs_do_writepages(mapping, &wbc, is_dir);
>> + wbc_detach_inode(&wbc);
>> + return ret;
>> +}
>> +
>> static void f2fs_write_failed(struct address_space *mapping, loff_t to)
>> {
>> struct inode *inode = mapping->host;
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 9280283..ea9bebb 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -2572,6 +2572,7 @@ void f2fs_invalidate_page(struct page *page, unsigned int offset,
>> int f2fs_migrate_page(struct address_space *mapping, struct page *newpage,
>> struct page *page, enum migrate_mode mode);
>> #endif
>> +int f2fs_filemap_fdatawrite(struct address_space *mapping, bool is_dir);
>>
>> /*
>> * gc.c
>> --
>> 1.8.5.2
> .
>
--
Thanks,
Yunlong Song
Powered by blists - more mailing lists