[PATCH 1/2] f2fs: detect host-managed SMR by feature flag

[PATCH 1/2] f2fs: detect host-managed SMR by feature flag

[Jaegeuk Kim]

If mkfs.f2fs gives a feature flag for host-managed SMR, we can set mode=lfs
by default.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/f2fs.h  | 21 +++++++++++++++++++++
 fs/f2fs/super.c | 14 ++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 32be19e..1a19c02 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -133,6 +133,7 @@ struct f2fs_mount_info {
 };
 
 #define F2FS_FEATURE_ENCRYPT	0x0001
+#define F2FS_FEATURE_HMSMR	0x0002
 
 #define F2FS_HAS_FEATURE(sb, mask)					\
 	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -2354,6 +2355,26 @@ static inline int f2fs_sb_has_crypto(struct super_block *sb)
 	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_ENCRYPT);
 }
 
+static inline int f2fs_sb_mounted_hmsmr(struct super_block *sb)
+{
+	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_HMSMR);
+}
+
+static inline void set_opt_mode(struct f2fs_sb_info *sbi, unsigned int mt)
+{
+	clear_opt(sbi, ADAPTIVE);
+	clear_opt(sbi, LFS);
+
+	switch (mt) {
+	case F2FS_MOUNT_ADAPTIVE:
+		set_opt(sbi, ADAPTIVE);
+		break;
+	case F2FS_MOUNT_LFS:
+		set_opt(sbi, LFS);
+		break;
+	}
+}
+
 static inline bool f2fs_may_encrypt(struct inode *inode)
 {
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index edc736d..71b6066 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -515,12 +515,10 @@ static int parse_options(struct super_block *sb, char *options)
 				return -ENOMEM;
 			if (strlen(name) == 8 &&
 					!strncmp(name, "adaptive", 8)) {
-				set_opt(sbi, ADAPTIVE);
-				clear_opt(sbi, LFS);
+				set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
 			} else if (strlen(name) == 3 &&
 					!strncmp(name, "lfs", 3)) {
-				clear_opt(sbi, ADAPTIVE);
-				set_opt(sbi, LFS);
+				set_opt_mode(sbi, F2FS_MOUNT_LFS);
 			} else {
 				kfree(name);
 				return -EINVAL;
@@ -980,7 +978,10 @@ static void default_options(struct f2fs_sb_info *sbi)
 	set_opt(sbi, EXTENT_CACHE);
 	sbi->sb->s_flags |= MS_LAZYTIME;
 	set_opt(sbi, FLUSH_MERGE);
-	set_opt(sbi, ADAPTIVE);
+	if (f2fs_sb_mounted_hmsmr(sbi->sb))
+		set_opt_mode(sbi, F2FS_MOUNT_LFS);
+	else
+		set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
 
 #ifdef CONFIG_F2FS_FS_XATTR
 	set_opt(sbi, XATTR_USER);
@@ -1629,6 +1630,8 @@ try_onemore:
 		goto free_sbi;
 
 	sb->s_fs_info = sbi;
+	sbi->raw_super = raw_super;
+
 	default_options(sbi);
 	/* parse mount options */
 	options = kstrdup((const char *)data, GFP_KERNEL);
@@ -1658,7 +1661,6 @@ try_onemore:
 	memcpy(sb->s_uuid, raw_super->uuid, sizeof(raw_super->uuid));
 
 	/* init f2fs-specific super block info */
-	sbi->raw_super = raw_super;
 	sbi->valid_super_block = valid_super_block;
 	mutex_init(&sbi->gc_mutex);
 	mutex_init(&sbi->cp_mutex);
-- 
2.8.3

[PATCH 2/2] f2fs: call update_inode_page for orphan inodes

[Jaegeuk Kim]

Let's store orphan inode pages right away.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c |  6 +++---
 fs/f2fs/dir.c        |  2 +-
 fs/f2fs/f2fs.h       |  2 +-
 fs/f2fs/inode.c      |  5 +++--
 fs/f2fs/namei.c      |  4 ++--
 fs/f2fs/super.c      | 16 +---------------
 6 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index f671683..afc03cb 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -508,10 +508,11 @@ void release_orphan_inode(struct f2fs_sb_info *sbi)
 	spin_unlock(&im->ino_lock);
 }
 
-void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
+void add_orphan_inode(struct inode *inode)
 {
 	/* add new orphan ino entry into list */
-	__add_ino_entry(sbi, ino, ORPHAN_INO);
+	__add_ino_entry(F2FS_I_SB(inode), inode->i_ino, ORPHAN_INO);
+	update_inode_page(inode);
 }
 
 void remove_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
@@ -535,7 +536,6 @@ static int recover_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino)
 	}
 
 	clear_nlink(inode);
-	mark_inode_dirty_sync(inode);
 
 	/* truncate all the data during iput */
 	iput(inode);
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 72a0207..7ba52a0 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -666,7 +666,7 @@ void f2fs_drop_nlink(struct inode *dir, struct inode *inode)
 	up_write(&F2FS_I(inode)->i_sem);
 
 	if (inode->i_nlink == 0)
-		add_orphan_inode(sbi, inode->i_ino);
+		add_orphan_inode(inode);
 	else
 		release_orphan_inode(sbi);
 }
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 1a19c02..3019286 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2061,7 +2061,7 @@ bool exist_written_data(struct f2fs_sb_info *, nid_t, int);
 int f2fs_sync_inode_meta(struct f2fs_sb_info *);
 int acquire_orphan_inode(struct f2fs_sb_info *);
 void release_orphan_inode(struct f2fs_sb_info *);
-void add_orphan_inode(struct f2fs_sb_info *, nid_t);
+void add_orphan_inode(struct inode *);
 void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
 int recover_orphan_inodes(struct f2fs_sb_info *);
 int get_valid_checkpoint(struct f2fs_sb_info *);
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 63c4326..9dd6642 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -394,7 +394,8 @@ no_delete:
 out_clear:
 	fscrypt_put_encryption_info(inode, NULL);
 
-	f2fs_bug_on(sbi, is_inode_flag_set(inode, FI_DIRTY_INODE));
+	f2fs_bug_on(sbi, !f2fs_cp_error(sbi) &&
+			is_inode_flag_set(inode, FI_DIRTY_INODE));
 	clear_inode(inode);
 }
 
@@ -421,7 +422,7 @@ void handle_failed_inode(struct inode *inode)
 			f2fs_msg(sbi->sb, KERN_WARNING,
 				"Too many orphan inodes, run fsck to fix.");
 		} else {
-			add_orphan_inode(sbi, inode->i_ino);
+			add_orphan_inode(inode);
 		}
 		alloc_nid_done(sbi, inode->i_ino);
 	} else {
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 618829e..4460400 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -598,7 +598,7 @@ static int __f2fs_tmpfile(struct inode *dir, struct dentry *dentry,
 	 * add this non-linked tmpfile to orphan list, in this way we could
 	 * remove all unused data of tmpfile after abnormal power-off.
 	 */
-	add_orphan_inode(sbi, inode->i_ino);
+	add_orphan_inode(inode);
 	alloc_nid_done(sbi, inode->i_ino);
 
 	if (whiteout) {
@@ -712,7 +712,7 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		up_write(&F2FS_I(new_inode)->i_sem);
 
 		if (!new_inode->i_nlink)
-			add_orphan_inode(sbi, new_inode->i_ino);
+			add_orphan_inode(new_inode);
 		else
 			release_orphan_inode(sbi);
 	} else {
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 71b6066..d78e46b 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -583,8 +583,6 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb)
 
 static int f2fs_drop_inode(struct inode *inode)
 {
-	int ret;
-
 	/*
 	 * This is to avoid a deadlock condition like below.
 	 * writeback_single_inode(inode)
@@ -620,19 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
 		return 0;
 	}
 
-	ret = generic_drop_inode(inode);
-	if (is_inode_flag_set(inode, FI_DIRTY_INODE)) {
-		if (ret)
-			inode->i_state |= I_WILL_FREE;
-		spin_unlock(&inode->i_lock);
-
-		update_inode_page(inode);
-
-		spin_lock(&inode->i_lock);
-		if (ret)
-			inode->i_state &= ~I_WILL_FREE;
-	}
-	return ret;
+	return generic_drop_inode(inode);
 }
 
 /*
-- 
2.8.3

Re: [PATCH 1/2] f2fs: detect host-managed SMR by feature flag

[Damien Le Moal]

Jaegeuk,

On 6/15/16 03:45, Jaegeuk Kim wrote:
> If mkfs.f2fs gives a feature flag for host-managed SMR, we can set mode=lfs
> by default.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/f2fs.h  | 21 +++++++++++++++++++++
>  fs/f2fs/super.c | 14 ++++++++------
>  2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 32be19e..1a19c02 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -133,6 +133,7 @@ struct f2fs_mount_info {
>  };
>
>  #define F2FS_FEATURE_ENCRYPT	0x0001
> +#define F2FS_FEATURE_HMSMR	0x0002
>
>  #define F2FS_HAS_FEATURE(sb, mask)					\
>  	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
> @@ -2354,6 +2355,26 @@ static inline int f2fs_sb_has_crypto(struct super_block *sb)
>  	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_ENCRYPT);
>  }
>
> +static inline int f2fs_sb_mounted_hmsmr(struct super_block *sb)
> +{
> +	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_HMSMR);
> +}
> +
> +static inline void set_opt_mode(struct f2fs_sb_info *sbi, unsigned int mt)
> +{
> +	clear_opt(sbi, ADAPTIVE);
> +	clear_opt(sbi, LFS);
> +
> +	switch (mt) {
> +	case F2FS_MOUNT_ADAPTIVE:
> +		set_opt(sbi, ADAPTIVE);
> +		break;
> +	case F2FS_MOUNT_LFS:
> +		set_opt(sbi, LFS);
> +		break;
> +	}
> +}
> +
>  static inline bool f2fs_may_encrypt(struct inode *inode)
>  {
>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index edc736d..71b6066 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -515,12 +515,10 @@ static int parse_options(struct super_block *sb, char *options)
>  				return -ENOMEM;
>  			if (strlen(name) == 8 &&
>  					!strncmp(name, "adaptive", 8)) {
> -				set_opt(sbi, ADAPTIVE);
> -				clear_opt(sbi, LFS);
> +				set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
>  			} else if (strlen(name) == 3 &&
>  					!strncmp(name, "lfs", 3)) {
> -				clear_opt(sbi, ADAPTIVE);
> -				set_opt(sbi, LFS);
> +				set_opt_mode(sbi, F2FS_MOUNT_LFS);
>  			} else {
>  				kfree(name);
>  				return -EINVAL;
> @@ -980,7 +978,10 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	set_opt(sbi, EXTENT_CACHE);
>  	sbi->sb->s_flags |= MS_LAZYTIME;
>  	set_opt(sbi, FLUSH_MERGE);
> -	set_opt(sbi, ADAPTIVE);
> +	if (f2fs_sb_mounted_hmsmr(sbi->sb))
> +		set_opt_mode(sbi, F2FS_MOUNT_LFS);
> +	else
> +		set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);

Strictly speaking, host-aware drives would not require the LFS mode
as any zone of the disk can be randomly written and so the occasional 
in-place updates of the adaptive mode would be OK. However, randomly 
writing host-aware drives can lead to a lot of background activity on 
the drive side for internal management of the accumulated random writes, 
with a potential performance drop over time (this is highly dependent on 
the drive FW implementation though).

So I think it would be good to also enable the LFS mode by default for 
host-aware devices, unless the user has explicitly specified mount with 
adaptive mode (if the user "knows" that the drive FW handles very 
efficiently random writes).

So how about also introducing a F2FS_FEATURE_HASMR feature flag to 
handle these different cases ?

Also, I think that the DISCARD option must be enabled by default for 
HMSMR disks. Otherwise, zones write pointer will never get reset.
The same applies to HASMR devices mounted with the LFS mode.
(In any case, the discard handling does not look like it will always 
align to the device zone size, which will fail on a zoned disk (discard 
granularity is the zone size). I may be missing something though. Still 
checking.)

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Group, HGST Research,
HGST, a Western Digital company
Damien.LeMoal@hgst.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.hgst.com
Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:

This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH 1/2] f2fs: detect host-managed SMR by feature flag

[Jaegeuk Kim]

On Wed, Jun 15, 2016 at 04:21:13AM +0000, Damien Le Moal wrote:
> Jaegeuk,
> 
> On 6/15/16 03:45, Jaegeuk Kim wrote:
> > If mkfs.f2fs gives a feature flag for host-managed SMR, we can set mode=lfs
> > by default.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/f2fs.h  | 21 +++++++++++++++++++++
> >  fs/f2fs/super.c | 14 ++++++++------
> >  2 files changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 32be19e..1a19c02 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -133,6 +133,7 @@ struct f2fs_mount_info {
> >  };
> >
> >  #define F2FS_FEATURE_ENCRYPT	0x0001
> > +#define F2FS_FEATURE_HMSMR	0x0002
> >
> >  #define F2FS_HAS_FEATURE(sb, mask)					\
> >  	((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
> > @@ -2354,6 +2355,26 @@ static inline int f2fs_sb_has_crypto(struct super_block *sb)
> >  	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_ENCRYPT);
> >  }
> >
> > +static inline int f2fs_sb_mounted_hmsmr(struct super_block *sb)
> > +{
> > +	return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_HMSMR);
> > +}
> > +
> > +static inline void set_opt_mode(struct f2fs_sb_info *sbi, unsigned int mt)
> > +{
> > +	clear_opt(sbi, ADAPTIVE);
> > +	clear_opt(sbi, LFS);
> > +
> > +	switch (mt) {
> > +	case F2FS_MOUNT_ADAPTIVE:
> > +		set_opt(sbi, ADAPTIVE);
> > +		break;
> > +	case F2FS_MOUNT_LFS:
> > +		set_opt(sbi, LFS);
> > +		break;
> > +	}
> > +}
> > +
> >  static inline bool f2fs_may_encrypt(struct inode *inode)
> >  {
> >  #ifdef CONFIG_F2FS_FS_ENCRYPTION
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index edc736d..71b6066 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -515,12 +515,10 @@ static int parse_options(struct super_block *sb, char *options)
> >  				return -ENOMEM;
> >  			if (strlen(name) == 8 &&
> >  					!strncmp(name, "adaptive", 8)) {
> > -				set_opt(sbi, ADAPTIVE);
> > -				clear_opt(sbi, LFS);
> > +				set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
> >  			} else if (strlen(name) == 3 &&
> >  					!strncmp(name, "lfs", 3)) {
> > -				clear_opt(sbi, ADAPTIVE);
> > -				set_opt(sbi, LFS);
> > +				set_opt_mode(sbi, F2FS_MOUNT_LFS);
> >  			} else {
> >  				kfree(name);
> >  				return -EINVAL;
> > @@ -980,7 +978,10 @@ static void default_options(struct f2fs_sb_info *sbi)
> >  	set_opt(sbi, EXTENT_CACHE);
> >  	sbi->sb->s_flags |= MS_LAZYTIME;
> >  	set_opt(sbi, FLUSH_MERGE);
> > -	set_opt(sbi, ADAPTIVE);
> > +	if (f2fs_sb_mounted_hmsmr(sbi->sb))
> > +		set_opt_mode(sbi, F2FS_MOUNT_LFS);
> > +	else
> > +		set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
> 
> Strictly speaking, host-aware drives would not require the LFS mode
> as any zone of the disk can be randomly written and so the occasional 
> in-place updates of the adaptive mode would be OK. However, randomly 
> writing host-aware drives can lead to a lot of background activity on 
> the drive side for internal management of the accumulated random writes, 
> with a potential performance drop over time (this is highly dependent on 
> the drive FW implementation though).
> 
> So I think it would be good to also enable the LFS mode by default for 
> host-aware devices, unless the user has explicitly specified mount with 
> adaptive mode (if the user "knows" that the drive FW handles very 
> efficiently random writes).

Agreed. Would HASMR also provide the same geometry, zone information, in mkfs?
Cause I haven't tested it so far.

IIRC, we discussed about HASMR with Marc Lehmann last year, and he gave the
below link which sums up.

http://comments.gmane.org/gmane.linux.file-systems.f2fs/2854
http://blog.schmorp.de/2015-10-08-smr-archive-drives-fast-now.html

> So how about also introducing a F2FS_FEATURE_HASMR feature flag to 
> handle these different cases ?

Yes, so once I can retrieve the zone information from the device, surely I'll
add that.

> Also, I think that the DISCARD option must be enabled by default for 
> HMSMR disks. Otherwise, zones write pointer will never get reset.
> The same applies to HASMR devices mounted with the LFS mode.
> (In any case, the discard handling does not look like it will always 
> align to the device zone size, which will fail on a zoned disk (discard 
> granularity is the zone size). I may be missing something though. Still 
> checking.)

Yup, will do. BTW, I remember I fixed zone-aligned discard issue before.
Could you please check the latest dev-test branch in f2fs.git?
I'm used to rebase that branch occasionally when I changed the previous patches.

Thanks,

> 
> Best regards.
> 
> -- 
> Damien Le Moal, Ph.D.
> Sr. Manager, System Software Group, HGST Research,
> HGST, a Western Digital company
> Damien.LeMoal@hgst.com
> (+81) 0466-98-3593 (ext. 513593)
> 1 kirihara-cho, Fujisawa,
> Kanagawa, 252-0888 Japan
> www.hgst.com
> Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer:
> 
> This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system.

Re: [PATCH 1/2] f2fs: detect host-managed SMR by feature flag

[Shaun Tancheff]

Hi Jaegeuk,

On Wed, Jun 15, 2016 at 11:32 AM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> On Wed, Jun 15, 2016 at 04:21:13AM +0000, Damien Le Moal wrote:
>> On 6/15/16 03:45, Jaegeuk Kim wrote:
>> > If mkfs.f2fs gives a feature flag for host-managed SMR, we can set mode=lfs
>> > by default.
>> >
>> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>> > ---
>> >  fs/f2fs/f2fs.h  | 21 +++++++++++++++++++++
>> >  fs/f2fs/super.c | 14 ++++++++------
>> >  2 files changed, 29 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> > index 32be19e..1a19c02 100644
>> > --- a/fs/f2fs/f2fs.h
>> > +++ b/fs/f2fs/f2fs.h
>> > @@ -133,6 +133,7 @@ struct f2fs_mount_info {
>> >  };
>> >
>> >  #define F2FS_FEATURE_ENCRYPT       0x0001
>> > +#define F2FS_FEATURE_HMSMR 0x0002
>> >
>> >  #define F2FS_HAS_FEATURE(sb, mask)                                 \
>> >     ((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
>> > @@ -2354,6 +2355,26 @@ static inline int f2fs_sb_has_crypto(struct super_block *sb)
>> >     return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_ENCRYPT);
>> >  }
>> >
>> > +static inline int f2fs_sb_mounted_hmsmr(struct super_block *sb)
>> > +{
>> > +   return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_HMSMR);
>> > +}
>> > +
>> > +static inline void set_opt_mode(struct f2fs_sb_info *sbi, unsigned int mt)
>> > +{
>> > +   clear_opt(sbi, ADAPTIVE);
>> > +   clear_opt(sbi, LFS);
>> > +
>> > +   switch (mt) {
>> > +   case F2FS_MOUNT_ADAPTIVE:
>> > +           set_opt(sbi, ADAPTIVE);
>> > +           break;
>> > +   case F2FS_MOUNT_LFS:
>> > +           set_opt(sbi, LFS);
>> > +           break;
>> > +   }
>> > +}
>> > +
>> >  static inline bool f2fs_may_encrypt(struct inode *inode)
>> >  {
>> >  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> > index edc736d..71b6066 100644
>> > --- a/fs/f2fs/super.c
>> > +++ b/fs/f2fs/super.c
>> > @@ -515,12 +515,10 @@ static int parse_options(struct super_block *sb, char *options)
>> >                             return -ENOMEM;
>> >                     if (strlen(name) == 8 &&
>> >                                     !strncmp(name, "adaptive", 8)) {
>> > -                           set_opt(sbi, ADAPTIVE);
>> > -                           clear_opt(sbi, LFS);
>> > +                           set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
>> >                     } else if (strlen(name) == 3 &&
>> >                                     !strncmp(name, "lfs", 3)) {
>> > -                           clear_opt(sbi, ADAPTIVE);
>> > -                           set_opt(sbi, LFS);
>> > +                           set_opt_mode(sbi, F2FS_MOUNT_LFS);
>> >                     } else {
>> >                             kfree(name);
>> >                             return -EINVAL;
>> > @@ -980,7 +978,10 @@ static void default_options(struct f2fs_sb_info *sbi)
>> >     set_opt(sbi, EXTENT_CACHE);
>> >     sbi->sb->s_flags |= MS_LAZYTIME;
>> >     set_opt(sbi, FLUSH_MERGE);
>> > -   set_opt(sbi, ADAPTIVE);
>> > +   if (f2fs_sb_mounted_hmsmr(sbi->sb))
>> > +           set_opt_mode(sbi, F2FS_MOUNT_LFS);
>> > +   else
>> > +           set_opt_mode(sbi, F2FS_MOUNT_ADAPTIVE);
>>
>> Strictly speaking, host-aware drives would not require the LFS mode
>> as any zone of the disk can be randomly written and so the occasional
>> in-place updates of the adaptive mode would be OK. However, randomly
>> writing host-aware drives can lead to a lot of background activity on
>> the drive side for internal management of the accumulated random writes,
>> with a potential performance drop over time (this is highly dependent on
>> the drive FW implementation though).
>>
>> So I think it would be good to also enable the LFS mode by default for
>> host-aware devices, unless the user has explicitly specified mount with
>> adaptive mode (if the user "knows" that the drive FW handles very
>> efficiently random writes).
>
> Agreed. Would HASMR also provide the same geometry, zone information, in mkfs?
> Cause I haven't tested it so far.

Yes. Host Aware SMR reports geometry in the same way. As far as I can tell,
your current mkfs for f2fs works correctly for HASMR.

> IIRC, we discussed about HASMR with Marc Lehmann last year, and he gave the
> below link which sums up.
>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__comments.gmane.org_gmane.linux.file-2Dsystems.f2fs_2854&d=CwIBAg&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ugl8V50qIHLe856QW0qfG3WVYGOrWzA&m=3Uw29rZTMJsKnUE9bBduFOdp3OiEwJhpgbJ9GmyO_SU&s=YNxrXKMJpd-BPHnYG8ctqg6ct58p2yqO3BRKlbjBO_s&e=
> https://urldefense.proofpoint.com/v2/url?u=http-3A__blog.schmorp.de_2015-2D10-2D08-2Dsmr-2Darchive-2Ddrives-2Dfast-2Dnow.html&d=CwIBAg&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ugl8V50qIHLe856QW0qfG3WVYGOrWzA&m=3Uw29rZTMJsKnUE9bBduFOdp3OiEwJhpgbJ9GmyO_SU&s=SC_RkdkfDQZTeIccq0o1zE4siI_RgDL4UhiVc1VzENY&e=

I think above applies to 'drive managed' SMR where the upper level
has no insight into the underlying zone layout.

>> So how about also introducing a F2FS_FEATURE_HASMR feature flag to
>> handle these different cases ?
>
> Yes, so once I can retrieve the zone information from the device, surely I'll
> add that.

I have posted some patches that will allow you to retrieve the zone
information for both HA and HM drives.

Can you see if this would meet your needs?

The data is returned to the BIO is in the same (drive) format as SG_IO.

>> Also, I think that the DISCARD option must be enabled by default for
>> HMSMR disks. Otherwise, zones write pointer will never get reset.
>> The same applies to HASMR devices mounted with the LFS mode.
>> (In any case, the discard handling does not look like it will always
>> align to the device zone size, which will fail on a zoned disk (discard
>> granularity is the zone size). I may be missing something though. Still
>> checking.)
>
> Yup, will do. BTW, I remember I fixed zone-aligned discard issue before.
> Could you please check the latest dev-test branch in f2fs.git?
> I'm used to rebase that branch occasionally when I changed the previous patches.

I think you will need to have the have access to the zone information.

Regards,
Shaun Tancheff