Re: [f2fs-dev] [PATCH v2 4/4] fsck.f2fs: Check write pointer consistency with valid blocks count

2019-08-26 Thread Chao Yu
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 
> ---
>  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?

BTW, how you think enabling f2fs kernel module to recover incorrect write
pointer of zone? Once f2fs-tools doesn't upgrade, however kernel does...

Thanks,

> +  */
> + if (c.fix_on) {
> + FIX_MSG("Reset write pointer at segment 0x%x",
> + zone_segno);
> + ret = f2fs_reset_zone(dev, blkz);
> + if (ret)
> + return ret;
> + fsck->chk.wp_fixed_zones++;
> + } else {
> + MSG(0, "Inconsistent write pointer at segment 0x%x\n",
> + zone_segno);
> + fsck->chk.wp_inconsistent_zones++;
> + }
>   return 0;
> + }
>  
>   /* check write pointer consistency with the curseg in the zone */
>   cs_block = START_BLOCK(sbi, cs->segno) + cs->next_blkoff;
> 


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments

2019-08-26 Thread Chao Yu
On 2019/8/27 10:01, Chao Yu wrote:
> On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
>> On sudden f2fs shutdown, zoned block device status and f2fs current
>> segment positions in meta data can be inconsistent. When f2fs shutdown
>> happens before write operations completes, write pointers of zoned block
>> devices can go further but f2fs meta data keeps current segments at
>> positions before the write operations. After remounting the f2fs, the
>> inconsistency causes write operations not at write pointers and
>> "Unaligned write command" error is reported. This error was observed when
>> xfstests test case generic/388 was run with f2fs on a zoned block device.
>>
>> To avoid the error, have f2fs.fsck check consistency between each current
>> segment's position and the write pointer of the zone the current segment
>> points to. If the write pointer goes advance from the current segment,
>> fix the current segment position setting at same as the write pointer
>> position. In case the write pointer is behind the current segment, write
>> zero data at the write pointer position to make write pointer position at
>> same as the current segment.
>>
>> When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
>> ask users to fix them or not. When inconsistencies get fixed, turn on
>> 'force' flag in fsck_verify() to enforce fixes in following checks. This
>> position fix is done at the beginning of do_fsck() function so that other
>> checks reflect the current segment modification.
>>
>> Signed-off-by: Shin'ichiro Kawasaki 
>> ---
>>  fsck/fsck.c | 133 
>>  fsck/fsck.h |   3 ++
>>  fsck/main.c |   2 +
>>  3 files changed, 138 insertions(+)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 8953ca1..21a06ac 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -2574,6 +2574,125 @@ out:
>>  return cnt;
>>  }
>>  
>> +struct write_pointer_check_data {
>> +struct f2fs_sb_info *sbi;
>> +struct device_info *dev;
>> +};
>> +
>> +#define SECTOR_SHIFT 9
>> +
>> +static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void 
>> *opaque)
>> +{
>> +struct write_pointer_check_data *wpd = opaque;
>> +struct f2fs_sb_info *sbi = wpd->sbi;
>> +struct device_info *dev = wpd->dev;
>> +struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
>> +block_t zone_block, wp_block, wp_blkoff, cs_block, b;
>> +unsigned int zone_segno, wp_segno;
>> +struct seg_entry *se;
>> +struct curseg_info *cs;
>> +int cs_index, ret;
>> +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;
>> +
>> +if (blk_zone_conv(blkz))
>> +return 0;
>> +
>> +zone_block = dev->start_blkaddr
>> ++ (blk_zone_sector(blkz) >> log_sectors_per_block);
>> +zone_segno = GET_SEGNO(sbi, zone_block);
>> +wp_block = dev->start_blkaddr
>> ++ (blk_zone_wp_sector(blkz) >> log_sectors_per_block);
>> +wp_segno = GET_SEGNO(sbi, wp_block);
>> +wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
>> +
>> +/* find the curseg which points to the zone */
>> +for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
>> +cs = _I(sbi)->curseg_array[cs_index];
>> +if (zone_segno <= cs->segno &&
>> +cs->segno < zone_segno + segs_per_zone)
>> +break;
>> +}
> 
> Will this happen?
> 
> - write checkpoint
> - curseg points zone A
> - write large number of data
> - curseg points zone B, write pointer > 0
> - sudden power cut, curseg will be reset to zone A
> 
> zone B's write pointer won't be verified due to curseg points to zone A?

IIUC, we are trying fix such condition in a separated PATCH 4/4.

Reviewed-by: Chao Yu 

Thanks


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v2 3/4] fsck.f2fs: Check write pointer consistency with current segments

2019-08-26 Thread Chao Yu
On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
> On sudden f2fs shutdown, zoned block device status and f2fs current
> segment positions in meta data can be inconsistent. When f2fs shutdown
> happens before write operations completes, write pointers of zoned block
> devices can go further but f2fs meta data keeps current segments at
> positions before the write operations. After remounting the f2fs, the
> inconsistency causes write operations not at write pointers and
> "Unaligned write command" error is reported. This error was observed when
> xfstests test case generic/388 was run with f2fs on a zoned block device.
> 
> To avoid the error, have f2fs.fsck check consistency between each current
> segment's position and the write pointer of the zone the current segment
> points to. If the write pointer goes advance from the current segment,
> fix the current segment position setting at same as the write pointer
> position. In case the write pointer is behind the current segment, write
> zero data at the write pointer position to make write pointer position at
> same as the current segment.
> 
> When inconsistencies are found, turn on c.bug_on flag in fsck_verify() to
> ask users to fix them or not. When inconsistencies get fixed, turn on
> 'force' flag in fsck_verify() to enforce fixes in following checks. This
> position fix is done at the beginning of do_fsck() function so that other
> checks reflect the current segment modification.
> 
> Signed-off-by: Shin'ichiro Kawasaki 
> ---
>  fsck/fsck.c | 133 
>  fsck/fsck.h |   3 ++
>  fsck/main.c |   2 +
>  3 files changed, 138 insertions(+)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 8953ca1..21a06ac 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -2574,6 +2574,125 @@ out:
>   return cnt;
>  }
>  
> +struct write_pointer_check_data {
> + struct f2fs_sb_info *sbi;
> + struct device_info *dev;
> +};
> +
> +#define SECTOR_SHIFT 9
> +
> +static int fsck_chk_write_pointer(int i, struct blk_zone *blkz, void *opaque)
> +{
> + struct write_pointer_check_data *wpd = opaque;
> + struct f2fs_sb_info *sbi = wpd->sbi;
> + struct device_info *dev = wpd->dev;
> + struct f2fs_fsck *fsck = F2FS_FSCK(sbi);
> + block_t zone_block, wp_block, wp_blkoff, cs_block, b;
> + unsigned int zone_segno, wp_segno;
> + struct seg_entry *se;
> + struct curseg_info *cs;
> + int cs_index, ret;
> + 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;
> +
> + if (blk_zone_conv(blkz))
> + return 0;
> +
> + zone_block = dev->start_blkaddr
> + + (blk_zone_sector(blkz) >> log_sectors_per_block);
> + zone_segno = GET_SEGNO(sbi, zone_block);
> + wp_block = dev->start_blkaddr
> + + (blk_zone_wp_sector(blkz) >> log_sectors_per_block);
> + wp_segno = GET_SEGNO(sbi, wp_block);
> + wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
> +
> + /* find the curseg which points to the zone */
> + for (cs_index = 0; cs_index < NO_CHECK_TYPE; cs_index++) {
> + cs = _I(sbi)->curseg_array[cs_index];
> + if (zone_segno <= cs->segno &&
> + cs->segno < zone_segno + segs_per_zone)
> + break;
> + }

Will this happen?

- write checkpoint
- curseg points zone A
- write large number of data
- curseg points zone B, write pointer > 0
- sudden power cut, curseg will be reset to zone A

zone B's write pointer won't be verified due to curseg points to zone A?

Thanks,

> +
> + if (cs_index >= NR_CURSEG_TYPE)
> + return 0;
> +
> + /* check write pointer consistency with the curseg in the zone */
> + cs_block = START_BLOCK(sbi, cs->segno) + cs->next_blkoff;
> + if (wp_block == cs_block)
> + return 0;
> +
> + if (!c.fix_on) {
> + MSG(0, "Inconsistent write pointer: "
> + "curseg %d[0x%x,0x%x] wp[0x%x,0x%x]\n",
> + cs_index, cs->segno, cs->next_blkoff, wp_segno, wp_blkoff);
> + fsck->chk.wp_inconsistent_zones++;
> + return 0;
> + }
> +
> + /*
> +  * If the curseg is in advance from the write pointer, write zero to
> +  * move the write pointer forward to the same position as the curseg.
> +  */
> + if (wp_block < cs_block) {
> + ret = 0;
> + zero_blk = calloc(BLOCK_SZ, 1);
> + if (!zero_blk)
> + return -EINVAL;
> +
> + FIX_MSG("Advance write pointer to match with curseg %d: "
> + "[0x%x,0x%x]->[0x%x,0x%x]",
> + cs_index, wp_segno, wp_blkoff,
> + cs->segno, cs->next_blkoff);
> + for (b = wp_block; b < cs_block && !ret; b++)
> + ret = dev_write_block(zero_blk, b);
> +
> + 

Re: [f2fs-dev] [PATCH v2 2/4] libf2fs_zoned: Introduce f2fs_reset_zone() function

2019-08-26 Thread Chao Yu
On 2019/8/21 12:48, Shin'ichiro Kawasaki wrote:
> To allow fsck to reset a zone with inconsistent write pointer, introduce
> a helper function f2fs_reset_zone().
> 
> Signed-off-by: Shin'ichiro Kawasaki 

Reviewed-by: Chao Yu 

Thanks,


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v2 1/4] libf2fs_zoned: Introduce f2fs_report_zones() helper function

2019-08-26 Thread Chao Yu
On 2019/8/21 12:47, Shin'ichiro Kawasaki wrote:
> To prepare for write pointer consistency check by fsck, add
> f2fs_report_zones() helper function which calls REPORT ZONE command to
> get write pointer status. The function is added to lib/libf2fs_zoned
> which gathers zoned block device related functions.
> 
> To check write pointer consistency with f2fs meta data, fsck needs to
> refer both of reported zone information and f2fs super block structure
> "f2fs_sb_info". However, libf2fs_zoned does not import f2fs_sb_info. To
> keep f2fs_sb_info structure out of libf2fs_zoned, provide a callback
> function in fsck to f2fs_report_zones() and call it for each zone.
> 
> Signed-off-by: Shin'ichiro Kawasaki 
> ---
>  include/f2fs_fs.h   |  2 ++
>  lib/libf2fs_zoned.c | 57 +
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 0d9a036..abadd1b 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -1279,6 +1279,8 @@ blk_zone_cond_str(struct blk_zone *blkz)
>  
>  extern int f2fs_get_zoned_model(int);
>  extern int f2fs_get_zone_blocks(int);
> +typedef int (report_zones_cb_t)(int i, struct blk_zone *blkz, void *opaque);
> +extern int f2fs_report_zones(int, report_zones_cb_t *, void *);
>  extern int f2fs_check_zones(int);
>  extern int f2fs_reset_zones(int);
>  
> diff --git a/lib/libf2fs_zoned.c b/lib/libf2fs_zoned.c
> index af00b44..fc4974f 100644
> --- a/lib/libf2fs_zoned.c
> +++ b/lib/libf2fs_zoned.c
> @@ -193,6 +193,57 @@ int f2fs_get_zone_blocks(int i)
>  
>  #define F2FS_REPORT_ZONES_BUFSZ  524288
>  
> +int f2fs_report_zones(int j, report_zones_cb_t *report_zones_cb, void 
> *opaque)
> +{
> + struct device_info *dev = c.devices + j;
> + struct blk_zone_report *rep;
> + struct blk_zone *blkz;
> + unsigned int i, n = 0;
> + u_int64_t total_sectors = (dev->total_sectors * c.sector_size) >> 9;

Hi Shin'ichiro,

Could we use SECTOR_SHIFT instead?

> + u_int64_t sector = 0;
> + int ret = -1;
> +
> + rep = malloc(F2FS_REPORT_ZONES_BUFSZ);
> + if (!rep) {
> + ERR_MSG("No memory for report zones\n");
> + return -ENOMEM;
> + }
> +
> + while (sector < total_sectors) {
> +
> + /* Get zone info */
> + rep->sector = sector;
> + rep->nr_zones = (F2FS_REPORT_ZONES_BUFSZ - sizeof(struct 
> blk_zone_report))
> + / sizeof(struct blk_zone);
> +
> + ret = ioctl(dev->fd, BLKREPORTZONE, rep);
> + if (ret != 0) {
> + ret = -errno;
> + ERR_MSG("ioctl BLKREPORTZONE failed\n");
It's minor, it will be better to print errno here, since I didn't see we print
error no from caller.

> + goto out;
> + }
> +
> + if (!rep->nr_zones) {
> + ret = -EIO;
> + ERR_MSG("Unexpected ioctl BLKREPORTZONE result\n");
> + goto out;
> + }
> +
> + blkz = (struct blk_zone *)(rep + 1);
> + for (i = 0; i < rep->nr_zones && sector < total_sectors; i++) {

The condition looks like that block zones in rep may across end-of-device? Will
this really happen?

So I mean will "i < rep->nr_zones" be enough?

Thanks,

> + ret = report_zones_cb(n, blkz, opaque);
> + if (ret)
> + goto out;
> + sector = blk_zone_sector(blkz) + blk_zone_length(blkz);
> + n++;
> + blkz++;
> + }
> + }
> +out:
> + free(rep);
> + return ret;
> +}
> +
>  int f2fs_check_zones(int j)
>  {
>   struct device_info *dev = c.devices + j;
> @@ -372,6 +423,12 @@ out:
>  
>  #else
>  
> +int f2fs_report_zones(int j, report_zones_cb_t *report_zones_cb, void 
> *opaque)
> +{
> + ERR_MSG("%d: Zoned block devices are not supported\n", i);
> + return -1;
> +}
> +
>  int f2fs_get_zoned_model(int i)
>  {
>   struct device_info *dev = c.devices + i;
> 


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v4 3/8] block: blk-crypto for Inline Encryption

2019-08-26 Thread Jonathan Corbet
On Wed, 21 Aug 2019 00:57:09 -0700
Satya Tangirala  wrote:

> We introduce blk-crypto, which manages programming keyslots for struct
> bios. With blk-crypto, filesystems only need to call bio_crypt_set_ctx with
> the encryption key, algorithm and data_unit_num; they don't have to worry
> about getting a keyslot for each encryption context, as blk-crypto handles
> that. Blk-crypto also makes it possible for layered devices like device
> mapper to make use of inline encryption hardware.
> 
> Blk-crypto delegates crypto operations to inline encryption hardware when
> available, and also contains a software fallback to the kernel crypto API.
> For more details, refer to Documentation/block/blk-crypto.txt.

So that file doesn't seem to exist; did you mean inline-encryption.txt
here?

> Signed-off-by: Satya Tangirala 
> ---
>  Documentation/block/inline-encryption.txt | 186 ++
>  block/Kconfig |   2 +
>  block/Makefile|   3 +-
>  block/bio-crypt-ctx.c |   7 +-
>  block/bio.c   |   5 +
>  block/blk-core.c  |  11 +-
>  block/blk-crypto.c| 737 ++
>  include/linux/bio-crypt-ctx.h |   7 +
>  include/linux/blk-crypto.h|  47 ++
>  9 files changed, 1002 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/block/inline-encryption.txt
>  create mode 100644 block/blk-crypto.c
>  create mode 100644 include/linux/blk-crypto.h
> 
> diff --git a/Documentation/block/inline-encryption.txt 
> b/Documentation/block/inline-encryption.txt
> new file mode 100644
> index ..925611a5ea65
> --- /dev/null
> +++ b/Documentation/block/inline-encryption.txt

So we've been doing our best to get rid of .txt files in the documentation
tree.  I'd really be a lot happier if this were an RST file instead.  The
good news is that it's already 99% RST, so little would have to change.

See the info in Documentation/doc-guide for details.

> @@ -0,0 +1,186 @@
> +BLK-CRYPTO and KEYSLOT MANAGER
> +===
> +
> +CONTENTS
> +1. Objective
> +2. Constraints and notes
> +3. Design
> +4. Blk-crypto
> + 4-1 What does blk-crypto do on bio submission
> +5. Layered Devices
> +6. Future optimizations for layered devices

RST would generate this TOC for you, so you can take it out.

> +1. Objective
> +
> +
> +We want to support inline encryption (IE) in the kernel.
> +To allow for testing, we also want a crypto API fallback when actual
> +IE hardware is absent. We also want IE to work with layered devices
> +like dm and loopback (i.e. we want to be able to use the IE hardware
> +of the underlying devices if present, or else fall back to crypto API
> +en/decryption).
> +
> +
> +2. Constraints and notes
> +
> +
> +1) IE hardware have a limited number of “keyslots” that can be programmed

Some people get irate when they encounter non-ASCII characters in the docs;
that includes "smart quotes".

Also, s/have/has/

> +with an encryption context (key, algorithm, data unit size, etc.) at any 
> time.
> +One can specify a keyslot in a data request made to the device, and the
> +device will en/decrypt the data using the encryption context programmed into
> +that specified keyslot. When possible, we want to make multiple requests with
> +the same encryption context share the same keyslot.
> +
> +2) We need a way for filesystems to specify an encryption context to use for
> +en/decrypting a struct bio, and a device driver (like UFS) needs to be able
> +to use that encryption context when it processes the bio.
> +
> +3) We need a way for device drivers to expose their capabilities in a unified
> +way to the upper layers.
> +
> +
> +3. Design
> +=
> +
> +We add a struct bio_crypt_ctx to struct bio that can represent an
> +encryption context, because we need to be able to pass this encryption
> +context from the FS layer to the device driver to act upon.
> +
> +While IE hardware works on the notion of keyslots, the FS layer has no
> +knowledge of keyslots - it simply wants to specify an encryption context to
> +use while en/decrypting a bio.
> +
> +We introduce a keyslot manager (KSM) that handles the translation from
> +encryption contexts specified by the FS to keyslots on the IE hardware.

So...if this were RST, you could have directives to pull in the nice
kerneldoc comments you've already put into the source.

I'll stop here...presumably I've made my point by now :)

Thanks for documenting this subsystem!

jon


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v2 0/4] fsck: Check write pointers of zoned block devices

2019-08-26 Thread Chao Yu
On 2019/8/26 8:10, Damien Le Moal wrote:
> Chao,
> 
> On 2019/08/23 22:09, Chao Yu wrote:
>> Hi Damien,
>>
>> Do you have time to take a look at this patch set, and add a reviewed-by if 
>> it
>> is okay to you. :)
> 
> My apologies for not chiming in earlier. Shinichiro works in my group and I

Hi Damien,

I can guess you're in the same team. ;)

> asked him to work on this. We have gone through several iterations internally
> and this is the latest version.

I do believe zoned block codes change needs your review, due to my less
experience on such device. :P

> 
> So:
> 
> Reviewed-by: Damien Le Moal 

Thanks a lot for the review.

Thanks,

> 
>>
>> Thanks,
>>
>> On 2019-8-21 12:47, Shin'ichiro Kawasaki wrote:
>>> On sudden f2fs shutdown, zoned block device status and f2fs meta data can be
>>> inconsistent. When f2fs shutdown happens during write operations, write 
>>> pointers
>>> on the device go forward but the f2fs meta data does not reflect write 
>>> pointer
>>> progress. This inconsistency will eventually cause "Unaligned write command"
>>> error when restarting write operation after the next mount. This error can 
>>> be
>>> observed with xfstests test case generic/388, which enforces sudden shutdown
>>> during write operation and checks the file system recovery. Once the error
>>> happens because of the inconsistency, the file system requires fix. However,
>>> fsck.f2fs does not have a feature to check and fix it.
>>>
>>> This patch series adds a new feature to fsck.f2fs to check and fix the
>>> inconsistency. First and second patches add two functions which helps fsck 
>>> to
>>> call report zone and reset zone commands to zoned block devices. Third patch
>>> checks write pointers of zones that current segments recorded in meta data 
>>> point
>>> to. This covers the failure symptom observed with xfstests. The last patch
>>> checks write pointers of zones that current segments do not point to, which
>>> covers a potential failure scenario.
>>>
>>> This patch series depends on other patches for zoned block devices, then it
>>> conflicts with the master branch in f2fs-tools.git as of Aug/19/2019. It 
>>> can be
>>> applied without conflict to dev and dev-test branch tips.
>>>
>>> Changes from v1:
>>> * Fixed build failure on dev branch
>>>
>>> Shin'ichiro Kawasaki (4):
>>>   libf2fs_zoned: Introduce f2fs_report_zones() helper function
>>>   libf2fs_zoned: Introduce f2fs_reset_zone() function
>>>   fsck.f2fs: Check write pointer consistency with current segments
>>>   fsck.f2fs: Check write pointer consistency with valid blocks count
>>>
>>>  fsck/fsck.c | 161 
>>>  fsck/fsck.h |   3 +
>>>  fsck/main.c |   2 +
>>>  include/f2fs_fs.h   |   3 +
>>>  lib/libf2fs_zoned.c |  81 ++
>>>  5 files changed, 250 insertions(+)
>>>
>>
> 
> 


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] f2fs: dirty memory increasing during gc_urgent

2019-08-26 Thread Chao Yu
Hi Ju Hyung,

On 2019/8/25 19:06, Ju Hyung Park wrote:
> Hi Chao,
> 
> On Sat, Aug 24, 2019 at 12:52 AM Chao Yu  wrote:
>> It's not intentional, I failed to reproduce this issue, could you add some 
>> logs
>> to track why we stop urgent GC even there are still dirty segments?
> 
> I'm pretty sure you can reproduce this issue quite easily.

Oh, I just notice that my scope of data sample is too small.

> 
> I can see this happening on multiple devices including my workstation,
> laptop and my Android phone.
> 
> Here's a simple reproduction step:
> 1. Do `rm -rf * && git reset --hard` a few times under a Linux kernel Git
> 2. Do a sync
> 3. echo 1 > /sys/fs/f2fs/dev/gc_urgent_sleep_time
> 4. echo 1 > /sys/fs/f2fs/dev/gc_urgent
> 5. Once the number on "GC calls" doesn't change, look at "Dirty" under
> /sys/kernel/debug/f2fs/status. It's close to 0.
> 6. After doing a 'sync', "Dirty" increases a lot.
> 7. Remember the number on "GC calls" and run 3 and 4 again.
> 8. The number of "GC calls" increases by a few hundreds.

Thank for provided test script.

I found out that after data blocks migration, their parent dnodes will become
dirty, so that once we execute step 6), some node segments become dirty...

So after step 6), we can run 3), 4) and 6) again, "Dirty" will close to zero,
that's because node blocks migration will not dirty their parent
(indirect/didirect) nodes.

Thanks,

> 
> Thanks.
> .
> 


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel