On 2015/12/9 14:11, Chao Yu wrote:
> Hi,
>
>> -----Original Message-----
>> From: Jaegeuk Kim [mailto:jaeg...@kernel.org]
>> Sent: Wednesday, December 09, 2015 2:19 AM
>> To: Yunlei He
>> Cc: linux-f2fs-devel@lists.sourceforge.net; chao2...@samsung.com;
>> hebi...@huawei.com;
>> stuart...@huawei.com
>> Subject: Re: [f2fs-dev] [PATCH] back-up raw_super in sbi
>>
>> Hi Yunlei,
>>
>> On Tue, Dec 08, 2015 at 09:17:13PM +0800, Yunlei He wrote:
>>> write_checkpoint() tries to get cp_blkaddr from superblock buffer,
>>> if the buffer happen to be destroied by something else, it may
>
> Yunlei,
>
> You mean hacking in memory? could you share more about process of destroying?
I do some test on kernel version 3.10 like this:
with mounted f2fs partition, use dd to erase the first sb
dd if=/dev/zero of=/dev/sdx bs=4k count=1
then sync, and the system will panic.
the log added in function do_checkpoint show:
heyunlei:start_blk = 0
So maybe dd dirty the super block buffer stored in sbi.
But, in latest dev branch, it has no problem, I don't know why.
>
>>> bring in unpredictable effect on f2fs.
>>>
>>> this patch fix it by back-up a raw_super copy.
>>>
>>> Signed-off-by: Yunlei He <heyun...@huawei.com>
>>> ---
>>> fs/f2fs/checkpoint.c | 3 ++-
>>> fs/f2fs/f2fs.h | 10 ++++++++++
>>> fs/f2fs/node.c | 2 +-
>>> fs/f2fs/super.c | 12 ++++++++----
>>> 4 files changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index f661d80..1e342fd 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -276,13 +276,14 @@ long sync_meta_pages(struct f2fs_sb_info *sbi, enum
>>> page_type type,
>>> long nr_to_write)
>>> {
>>> struct address_space *mapping = META_MAPPING(sbi);
>>> - pgoff_t index = 0, end = LONG_MAX, prev = LONG_MAX;
>>> + pgoff_t index, end = LONG_MAX, prev = LONG_MAX;
>>> struct pagevec pvec;
>>> long nwritten = 0;
>>> struct writeback_control wbc = {
>>> .for_reclaim = 0,
>>> };
>>>
>>> + index = __start_cp_addr(sbi);
>>
>> No need to do this.
>>
>>> pagevec_init(&pvec, 0);
>>>
>>> while (index <= end) {
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 0052ae8..6afb56c 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1167,6 +1167,16 @@ static inline block_t __start_sum_addr(struct
>>> f2fs_sb_info *sbi)
>>> return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>>> }
>>>
>>> +static inline block_t __start_main_addr(struct f2fs_sb_info *sbi)
>>> +{
>>> + return le32_to_cpu(F2FS_RAW_SUPER(sbi)->main_blkaddr);
>>> +}
>>> +
>>> +static inline block_t __start_sum_addr(struct f2fs_sb_info *sbi)
>>> +{
>>> + return le32_to_cpu(F2FS_CKPT(sbi)->cp_pack_start_sum);
>>> +}
>>> +
>>
>> Ditto.
>>
>>> static inline bool inc_valid_node_count(struct f2fs_sb_info *sbi,
>>> struct inode *inode)
>>> {
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index 7bcbc6e..7d2b0bb 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -1161,7 +1161,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, nid_t
>>> ino,
>>> pagevec_init(&pvec, 0);
>>>
>>> next_step:
>>> - index = 0;
>>> + index = __start_main_addr(sbi);
>>
>> No, its index is for nid.
>>
>>> end = LONG_MAX;
>>>
>>> while (index <= end) {
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index bd7e9c6..d394711 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -1040,6 +1040,9 @@ static int read_raw_super_block(struct super_block
>>> *sb,
>>> struct f2fs_super_block *super;
>>> int err = 0;
>>>
>>> + super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
>>> + if (!super)
>>> + return -ENOMEM;
>>> retry:
>>> buffer = sb_bread(sb, block);
>>> if (!buffer) {
>>> @@ -1055,9 +1058,8 @@ retry:
>>> }
>>> }
>>>
>>> - super = (struct f2fs_super_block *)
>>> - ((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
>>> -
>>> + memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET,
>>> + sizeof(struct f2fs_super_block));
>>> /* sanity checking of raw super */
>>> if (sanity_check_raw_super(sb, super)) {
>>> brelse(buffer);
>>> @@ -1090,8 +1092,10 @@ retry:
>>>
>>> out:
>>> /* No valid superblock */
>>> - if (!*raw_super)
>>> + if (!*raw_super) {
>>> + kfree(super);
>>> return err;
>>> + }
>>
>> Need to consider f2fs_commit_super and kfree() in put_super.
>
> How about releasing grabbed block buffer 'sbi->raw_super_buf' since we
> already had one copy of it?
>
> Thanks,
>
>>
>> Could you check out this patch?
>>
>> ---
>> fs/f2fs/super.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index dbf16ad..8541c93 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -567,6 +567,7 @@ static void f2fs_put_super(struct super_block *sb)
>>
>> sb->s_fs_info = NULL;
>> brelse(sbi->raw_super_buf);
>> + kfree(sbi->raw_super);
>> kfree(sbi);
>> }
>>
>> @@ -1040,6 +1041,9 @@ static int read_raw_super_block(struct super_block *sb,
>> struct f2fs_super_block *super;
>> int err = 0;
>>
>> + super = kzalloc(sizeof(struct f2fs_super_block), GFP_KERNEL);
>> + if (!super)
>> + return -ENOMEM;
>> retry:
>> buffer = sb_bread(sb, block);
>> if (!buffer) {
>> @@ -1055,8 +1059,7 @@ retry:
>> }
>> }
>>
>> - super = (struct f2fs_super_block *)
>> - ((char *)(buffer)->b_data + F2FS_SUPER_OFFSET);
>> + memcpy(super, buffer->b_data + F2FS_SUPER_OFFSET, sizeof(*super));
>>
>> /* sanity checking of raw super */
>> if (sanity_check_raw_super(sb, super)) {
>> @@ -1090,14 +1093,17 @@ retry:
>>
>> out:
>> /* No valid superblock */
>> - if (!*raw_super)
>> + if (!*raw_super) {
>> + kfree(super);
>> return err;
>> + }
>>
>> return 0;
>> }
>>
>> int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover)
>> {
>> + struct f2fs_super_block *super = F2FS_RAW_SUPER(sbi);
>> struct buffer_head *sbh = sbi->raw_super_buf;
>> struct buffer_head *bh;
>> int err;
>> @@ -1108,7 +1114,7 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool
>> recover)
>> return -EIO;
>>
>> lock_buffer(bh);
>> - memcpy(bh->b_data, sbh->b_data, sbh->b_size);
>> + memcpy(bh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>> WARN_ON(sbh->b_size != F2FS_BLKSIZE);
>> set_buffer_uptodate(bh);
>> set_buffer_dirty(bh);
>> @@ -1124,6 +1130,10 @@ int f2fs_commit_super(struct f2fs_sb_info *sbi, bool
>> recover)
>>
>> /* write current valid superblock */
>> lock_buffer(sbh);
>> + if (memcmp(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super))) {
>> + f2fs_msg(sbi->sb, KERN_INFO, "Write modified valid superblock");
>> + memcpy(sbh->b_data + F2FS_SUPER_OFFSET, super, sizeof(*super));
>> + }
>> set_buffer_dirty(sbh);
>> unlock_buffer(sbh);
>>
>> --
>> 2.4.9 (Apple Git-60)
>
>
>
> .
>
------------------------------------------------------------------------------
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
