linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>,
	Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	Damien Le Moal <Damien.LeMoal@wdc.com>,
	"linux-f2fs-devel@lists.sourceforge.net"
	<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH v2 4/4] fsck.f2fs: Check write pointer consistency with valid blocks count
Date: Thu, 29 Aug 2019 22:42:03 +0800	[thread overview]
Message-ID: <b7be8727-ff87-7dd8-eb1f-89c0f9247213@kernel.org> (raw)
In-Reply-To: <20190828115333.ciivgtdmdprjxgaa@shindev.dhcp.fujisawa.hgst.com>

On 2019-8-28 19:53, Shinichiro Kawasaki wrote:
> On Aug 27, 2019 / 10:25, Chao Yu wrote:
>> On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
>>> When sudden f2fs shutdown happens on zoned block devices, write
>>> pointers can be inconsistent with valid blocks counts in meta data.
>>> The failure scenario is as follows:
>>>
>>> - Just before a sudden shutdown, a new segment in a new zone is selected
>>>   for a current segment. Write commands were executed to the segment.
>>>   and the zone has a write pointer not at zone start.
>>> - Before the write commands complete, shutdown happens. Meta data is
>>>   not updated and still keeps zero valid blocks count for the zone.
>>> - After next mount of the file system, the zone is selected for the next
>>>   write target because it has zero valid blocks count. However, it has
>>>   the write pointer not at zone start. Then "Unaligned write command"
>>>   error happens.
>>>
>>> To avoid this potential error path, reset write pointers if the zone
>>> does not have a current segment, the write pointer is not at the zone
>>> start and the zone has no valid blocks.
>>>
>>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>>> ---
>>>  fsck/fsck.c | 30 +++++++++++++++++++++++++++++-
>>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>>> index 21a06ac..cc9bbc0 100644
>>> --- a/fsck/fsck.c
>>> +++ b/fsck/fsck.c
>>> @@ -2595,6 +2595,7 @@ static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void *opaque)
>>>  	int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
>>>  	unsigned int segs_per_zone = sbi->segs_per_sec * sbi->secs_per_zone;
>>>  	void *zero_blk;
>>> +	block_t	zone_valid_blocks = 0;
>>>  
>>>  	if (blk_zone_conv(blkz))
>>>  		return 0;
>>> @@ -2615,8 +2616,35 @@ static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void *opaque)
>>>  			break;
>>>  	}
>>>  
>>> -	if (cs_index >= NR_CURSEG_TYPE)
>>> +	if (cs_index >= NR_CURSEG_TYPE) {
>>> +		for (b = zone_block; b < zone_block + c.zone_blocks &&
>>> +			     IS_VALID_BLK_ADDR(sbi, b); b += c.blks_per_seg) {
>>> +			se = get_seg_entry(sbi, GET_SEGNO(sbi, b));
>>> +			zone_valid_blocks += se->valid_blocks;
>>> +		}
>>> +		if (wp_block == zone_block || zone_valid_blocks)
>>> +			return 0;
>>> +
>>> +		/*
>>> +		 * The write pointer is not at zone start but there is no valid
>>> +		 * block in the zone. Segments in the zone can be selected for
>>> +		 * next write. Need to reset the write pointer to avoid
>>> +		 * unaligned write command error.
>>
>> In SPOR (sudden power-off recovery) of kernel side, we may revalidate blocks
>> belong to fsynced file in such zone within range of [0, write pointer], if we
>> just reset zone, will we lose those data for ever?
> 
> Yes. This patch resets zone and the data will be lost. I walked through
> fs/f2fs/recovery.c and learned that nodes with fsync mark are recovered at
> remount. Such fsync recovery cannot be done after zone reset. To avoid the
> data loss, I would like to drop this fourth patch at this moment.
> 
> Later on, I will consider safer approach not to reset the zone, but to set next
> write target block at the write pointer. I guess this approach will need kernel
> side patch to change block selection logic.

I guess below commit can help to recognize fsynced data in unclean umounted
image, maybe we can skip invalidating those data during zone write pointer recovery.

f2fs-tools: fix to skip block allocation for fsynced data

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test&id=a50cfc89e56ce8c022e295bf4de619af070fabe9

> 
>>
>> BTW, how you think enabling f2fs kernel module to recover incorrect write
>> pointer of zone? Once f2fs-tools doesn't upgrade, however kernel does...
> 
> Current f2fs allows to mount zoned block devices even when they have
> inconsistency with f2fs meta data. This is not good. Then I believe kernel side
> needs the feature to check write pointer inconsistency at mount time and fix it.
> 
> As you indicate, fix by kernel is more handy than notice to run fsck, especially
> when users do not have latest f2fs-tools. Still fix by fsck is needed when users
> use the kernel without the fix feature. I think both approaches are required:
> fix by kernel and fix by fsck.

Agreed, let's try to fix in both side.

Thanks,

> 
> --
> Best Regards,
> Shin'ichiro Kawasaki
> 
> _______________________________________________
> 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

  reply	other threads:[~2019-08-29 14:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-21  4:47 [f2fs-dev] [PATCH v2 0/4] fsck: Check write pointers of zoned block devices Shin'ichiro Kawasaki
2019-08-21  4:47 ` [f2fs-dev] [PATCH v2 1/4] libf2fs_zoned: Introduce f2fs_report_zones() helper function Shin'ichiro Kawasaki
2019-08-27  1:34   ` Chao Yu
2019-08-28  8:32     ` Shinichiro Kawasaki
2019-08-21  4:48 ` [f2fs-dev] [PATCH v2 2/4] libf2fs_zoned: Introduce f2fs_reset_zone() function Shin'ichiro Kawasaki
2019-08-27  1:36   ` Chao Yu
2019-08-21  4:48 ` [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments Shin'ichiro Kawasaki
2019-08-27  2:01   ` Chao Yu
2019-08-27  2:13     ` Chao Yu
2019-08-29  4:41       ` Shinichiro Kawasaki
2019-08-21  4:48 ` [f2fs-dev] [PATCH v2 4/4] fsck.f2fs: Check write pointer consistency with valid blocks count Shin'ichiro Kawasaki
2019-08-27  2:25   ` Chao Yu
2019-08-28 11:53     ` Shinichiro Kawasaki
2019-08-29 14:42       ` Chao Yu [this message]
2019-08-30  7:21         ` Shinichiro Kawasaki
2019-08-23 13:09 ` [f2fs-dev] [PATCH v2 0/4] fsck: Check write pointers of zoned block devices Chao Yu
2019-08-26  0:10   ` Damien Le Moal
2019-08-26  7:37     ` 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=b7be8727-ff87-7dd8-eb1f-89c0f9247213@kernel.org \
    --to=chao@kernel.org \
    --cc=Damien.LeMoal@wdc.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=shinichiro.kawasaki@wdc.com \
    --cc=yuchao0@huawei.com \
    /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).