[RFC] fs: New zonefs file system
diff mbox series

Message ID 20190712030017.14321-1-damien.lemoal@wdc.com
State New
Headers show
Series
  • [RFC] fs: New zonefs file system
Related show

Commit Message

Damien Le Moal July 12, 2019, 3 a.m. UTC
zonefs is a very simple file system exposing each zone of a zoned
block device as a file. This is intended to simplify implementation of
application zoned block device raw access support by allowing switching
to the well known POSIX file API rather than relying on direct block
device file ioctls and read/write. Zonefs, for instance, greatly
simplifies the implementation of LSM (log-structured merge) tree
structures (such as used in RocksDB and LevelDB) on zoned block devices
by allowing SSTables to be stored in a zone file similarly to a regular
file system architecture, hence reducing the amount of change needed in
the application.

Due to its simplicity, zonefs is in fact closer to a raw block device
access interface than to a full feature POSIX file system. This RFC
implementation uses Christoph's work on generic iomap writepage
implementation and is based on Christoph's gfs2 tree available at
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/gfs2-iomap

Zonefs on-disk metadata is reduced to a super block to store a magic
number, a uuid and optional features flags and values. On mount, zonefs
uses blkdev_report_zones() to obtain the device zone configuration and
populates the mount point with a static file tree solely based on this
information. E.g. file sizes come from zone write pointer offset managed
by the device itself.

The zone files created on mount have the following characteristics.
1) Files representing zones of the same type are grouped together
   under a common directory:
  * For conventional zones, the directory "cnv" is used.
  * For sequential write zones, the directory "seq" is used.
  These two directories are the only directories that exist in zonefs.
  Users cannot create other directories and cannot rename nor delete
  the "cnv" and "seq" directories.
2) The name of zone files is by default the number of the file within
   the zone type directory, in order of increasing zone start sector.
3) The size of conventional zone files is fixed to the device zone size.
   Conventional zone files cannot be truncated.
4) The size of sequential zone files represent the file zone write
   pointer position relative to the zone start sector. Truncating these
   files is allowed only down to 0, in wich case, the zone is reset to
   rewind the file zone write pointer position to the start of the zone.
5) All read and write operations to files are not allowed beyond the
   file zone size. Any access exceeding the zone size is failed with
   the -EFBIG error.
6) Creating, deleting, renaming or modifying any attribute of files
   and directories is not allowed. The only exception being the file
   size of sequential zone files which can be modified by write
   operations or truncation to 0.

Several optional features of zonefs can be enabled at format time.
* Conventional zone aggregation: contiguous conventional zones can be
  agregated into a single larger file instead of multiple per-zone
  files.
* File naming: the default file number file name can be switched to
  using the base-10 value of the file zone start sector.
* File ownership: The owner UID and GID of zone files is by default 0
  (root) but can be changed to any valid UID/GID.
* File access permissions: the default 640 access permissions can be
  changed.

The mkzonefs tool is used to format zonefs. This tool will be available
on Github at:

https://github.com/westerndigitalcorporation/zonefs-tools

Example: the following formats a host-managed SMR HDD with the
conventional zone aggregation feature enabled.

mkzonefs -o aggr_cnv /dev/sdX
mount -t zonefs /dev/sdX /mnt
ls -l /mnt/
total 0
dr-xr-xr-x 2 root root 0 Apr 11 13:00 cnv
dr-xr-xr-x 2 root root 0 Apr 11 13:00 seq

ls -l /mnt/cnv
total 137363456
-rw-rw---- 1 root root 140660178944 Apr 11 13:00 0

ls -Fal -v /mnt/seq
total 14511243264
dr-xr-xr-x 2 root root 15942528 Jul 10 11:53 ./
drwxr-xr-x 4 root root     1152 Jul 10 11:53 ../
-rw-r----- 1 root root        0 Jul 10 11:53 0
-rw-r----- 1 root root 33554432 Jul 10 13:43 1
-rw-r----- 1 root root        0 Jul 10 11:53 2
-rw-r----- 1 root root        0 Jul 10 11:53 3
...

The aggregated conventional zone file can be used as a regular file.
Operations such as the following work.

mkfs.ext4 /mnt/cnv/0
mount -o loop /mnt/cnv/0 /data

Contains contributions from Johannes Thumshirn <jthumshirn@suse.de>
and Christoph Hellwig <hch@lst.de>.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/Kconfig                 |    2 +
 fs/Makefile                |    1 +
 fs/zonefs/Kconfig          |    9 +
 fs/zonefs/Makefile         |    4 +
 fs/zonefs/super.c          | 1004 ++++++++++++++++++++++++++++++++++++
 fs/zonefs/zonefs.h         |  190 +++++++
 include/uapi/linux/magic.h |    1 +
 7 files changed, 1211 insertions(+)
 create mode 100644 fs/zonefs/Kconfig
 create mode 100644 fs/zonefs/Makefile
 create mode 100644 fs/zonefs/super.c
 create mode 100644 fs/zonefs/zonefs.h

Comments

Johannes Thumshirn July 12, 2019, 8 a.m. UTC | #1
On Fri, Jul 12, 2019 at 12:00:17PM +0900, Damien Le Moal wrote:

Hi Daminen,

Thanks for submitting zonefs.

Please find my first few comments, I'll have a second look later as well.

[...]

> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  fs/Kconfig                 |    2 +
>  fs/Makefile                |    1 +
>  fs/zonefs/Kconfig          |    9 +
>  fs/zonefs/Makefile         |    4 +
>  fs/zonefs/super.c          | 1004 ++++++++++++++++++++++++++++++++++++
>  fs/zonefs/zonefs.h         |  190 +++++++
>  include/uapi/linux/magic.h |    1 +
>  7 files changed, 1211 insertions(+)
>  create mode 100644 fs/zonefs/Kconfig
>  create mode 100644 fs/zonefs/Makefile
>  create mode 100644 fs/zonefs/super.c
>  create mode 100644 fs/zonefs/zonefs.h

It'll probably be good to add yourself as a maitainer in MAINTAINERS, so
people see there's a go-to person for patches. Also a list (fsdevel@) and a
git tree won't harm.

> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index f1046cf6ad85..e48cc0e0efbb 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -41,6 +41,7 @@ source "fs/ocfs2/Kconfig"
>  source "fs/btrfs/Kconfig"
>  source "fs/nilfs2/Kconfig"
>  source "fs/f2fs/Kconfig"
> +source "fs/zonefs/Kconfig"
>  
>  config FS_DAX
>  	bool "Direct Access (DAX) support"
> @@ -262,6 +263,7 @@ source "fs/romfs/Kconfig"
>  source "fs/pstore/Kconfig"
>  source "fs/sysv/Kconfig"
>  source "fs/ufs/Kconfig"
> +source "fs/ufs/Kconfig"
>  

This hunk looks wrong.

>  endif # MISC_FILESYSTEMS
>  

[...]

> +	/*
> +	 * Note: The first zone contains the super block: skip it.
> +	 */

I know I've been advocating for having on-disk metadata, but do we really
sacrifice a whole zone per default? I thought we'll have on-disk metadata
optional (I might be completely off the track here and need more coffee to
wake up though).

> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
> +	for (zone = &zones[1]; zone < end; zone = next) {

[...]

> +
> +	/* Set defaults */
> +	sbi->s_uid = GLOBAL_ROOT_UID;
> +	sbi->s_gid = GLOBAL_ROOT_GID;
> +	sbi->s_perm = S_IRUSR | S_IWUSR | S_IRGRP; /* 0640 */
> +
> +
> +	ret = zonefs_read_super(sb);
> +	if (ret)
> +		return ret;

That would be cool to be controllable via a mount option and have it:
	sbi->s_uid = opt.uid;
	sbi->s_gid = opt.gid;
	sbi->s_perm = opt.mode;

or pass these mount options to zonefs_read_super() and they can be set after
the feature validation.

> +
> +	zones = zonefs_get_zone_info(sb);
> +	if (IS_ERR(zones))
> +		return PTR_ERR(zones);
> +
> +	pr_info("zonefs: Mounting %s, %u zones",
> +		sb->s_id, sbi->s_nr_zones[ZONEFS_ZTYPE_ALL]);
> +
> +	/* Create root directory inode */
> +	ret = -ENOMEM;
> +	inode = new_inode(sb);
> +	if (!inode)
> +		goto out;

Nit: please add a blank line after the goto.

> +	inode->i_ino = get_next_ino();
> +	inode->i_mode = S_IFDIR | 0755;
> +	inode->i_ctime = inode->i_mtime = inode->i_atime = current_time(inode);
> +	inode->i_op = &simple_dir_inode_operations;
> +	inode->i_fop = &simple_dir_operations;
> +	inode->i_size = sizeof(struct dentry) * 2;
> +	set_nlink(inode, 2);

Nit: please add a blank line here as well.

> +	sb->s_root = d_make_root(inode);
> +	if (!sb->s_root)
> +		goto out;

[...]

> +/*
> + * Maximum length of file names: this only needs to be large enough to fit
> + * the zone group directory names and a decimal value of the start sector of
> + * the zones for file names. 16 characterse is plenty.
                           characters ^

[...]

> +struct zonefs_super {
> +
> +	/* Magic number */
> +	__le32		s_magic;		/*    4 */
> +
> +	/* Metadata version number */
> +	__le32		s_version;		/*    8 */
> +
> +	/* Features */
> +	__le64		s_features;		/*   16 */
> +
> +	/* 128-bit uuid */
> +	__u8		s_uuid[16];		/*   32 */
> +
> +	/* UID/GID to use for files */
> +	__le32		s_uid;			/*   36 */
> +	__le32		s_gid;			/*   40 */
> +
> +	/* File permissions */
> +	__le32		s_perm;			/*   44 */
> +
> +	/* Padding to 4K */
> +	__u8		s_reserved[4052];	/* 4096 */
> +
> +} __attribute__ ((packed));

I'm not sure the (end)offset comments are of any value here, it's nothing that
can't be obtained from pahole or gdb (or even by hand).

> +
> +/*
> + * Metadata version.
> + */
> +#define ZONEFS_VERSION	1
> +
> +/*
> + * Feature flags.
> + */
> +enum zonefs_features {
> +	/*
> +	 * Use a zone start sector value as file name.
> +	 */
> +	ZONEFS_F_STARTSECT_NAME,
> +	/*
> +	 * Aggregate contiguous conventional zones into a single file.
> +	 */
> +	ZONEFS_F_AGRCNV,
> +	/*
> +	 * Use super block specified UID for files instead of default.
> +	 */
> +	ZONEFS_F_UID,
> +	/*
> +	 * Use super block specified GID for files instead of default.
> +	 */
> +	ZONEFS_F_GID,
> +	/*
> +	 * Use super block specified file permissions instead of default 640.
> +	 */
> +	ZONEFS_F_PERM,
> +};

I'd rather not write the uid, gid, permissions and startsect name to the
superblock but have it controllable via a mount option. Just write the feature
to the superblock so we know we _can_ control this per mount.

Byte,
	Johannes
Damien Le Moal July 12, 2019, 8:31 a.m. UTC | #2
On 2019/07/12 17:00, Johannes Thumshirn wrote:
> On Fri, Jul 12, 2019 at 12:00:17PM +0900, Damien Le Moal wrote:
> 
> Hi Daminen,
> 
> Thanks for submitting zonefs.
> 
> Please find my first few comments, I'll have a second look later as well.
> 
> [...]
> 
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
>> ---
>>  fs/Kconfig                 |    2 +
>>  fs/Makefile                |    1 +
>>  fs/zonefs/Kconfig          |    9 +
>>  fs/zonefs/Makefile         |    4 +
>>  fs/zonefs/super.c          | 1004 ++++++++++++++++++++++++++++++++++++
>>  fs/zonefs/zonefs.h         |  190 +++++++
>>  include/uapi/linux/magic.h |    1 +
>>  7 files changed, 1211 insertions(+)
>>  create mode 100644 fs/zonefs/Kconfig
>>  create mode 100644 fs/zonefs/Makefile
>>  create mode 100644 fs/zonefs/super.c
>>  create mode 100644 fs/zonefs/zonefs.h
> 
> It'll probably be good to add yourself as a maitainer in MAINTAINERS, so
> people see there's a go-to person for patches. Also a list (fsdevel@) and a
> git tree won't harm.

Oops. Yes, indeed. I forgot to add that. I will.

> 
>>
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index f1046cf6ad85..e48cc0e0efbb 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -41,6 +41,7 @@ source "fs/ocfs2/Kconfig"
>>  source "fs/btrfs/Kconfig"
>>  source "fs/nilfs2/Kconfig"
>>  source "fs/f2fs/Kconfig"
>> +source "fs/zonefs/Kconfig"
>>  
>>  config FS_DAX
>>  	bool "Direct Access (DAX) support"
>> @@ -262,6 +263,7 @@ source "fs/romfs/Kconfig"
>>  source "fs/pstore/Kconfig"
>>  source "fs/sysv/Kconfig"
>>  source "fs/ufs/Kconfig"
>> +source "fs/ufs/Kconfig"
>>  
> 
> This hunk looks wrong.

Yes it is wrong. No idea how that sneaked in here. Will fix it.

> 
>>  endif # MISC_FILESYSTEMS
>>  
> 
> [...]
> 
>> +	/*
>> +	 * Note: The first zone contains the super block: skip it.
>> +	 */
> 
> I know I've been advocating for having on-disk metadata, but do we really
> sacrifice a whole zone per default? I thought we'll have on-disk metadata
> optional (I might be completely off the track here and need more coffee to
> wake up though).

Yes, indeed we do not really need the super block for now. But it is still super
useful to have so that:
1) libblkid and other such userland tools can probe the disk to see its format,
and preserve the usual "use -force option if you really want to overwrite"
behavior of all format tools.
2) Still related to previous point, the super block allows commands like:
mount /dev/sdX /mnt
and
mount -t zonefs /dev/sdX /mnt
to have the same result. That is, without the super block, if the drive was
previously formatted for btrfs or f2fs, the first command will mount that old
format, while the second will mount zonefs without necessarily erasing the old
FS super block.
3) Having the super block with a version number will allow in the future to add
more metadata (e.g. file names as decided by the application) while allowing
backward compatibility of the code.

>> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
>> +	for (zone = &zones[1]; zone < end; zone = next) {
> 
> [...]
> 
>> +
>> +	/* Set defaults */
>> +	sbi->s_uid = GLOBAL_ROOT_UID;
>> +	sbi->s_gid = GLOBAL_ROOT_GID;
>> +	sbi->s_perm = S_IRUSR | S_IWUSR | S_IRGRP; /* 0640 */
>> +
>> +
>> +	ret = zonefs_read_super(sb);
>> +	if (ret)
>> +		return ret;
> 
> That would be cool to be controllable via a mount option and have it:
> 	sbi->s_uid = opt.uid;
> 	sbi->s_gid = opt.gid;
> 	sbi->s_perm = opt.mode;
> 
> or pass these mount options to zonefs_read_super() and they can be set after
> the feature validation.

Yes, I thought of that and even had that implemented in a previous version. I
switched to the static format time definition only so that the resulting
operation of the FS is a little more like a normal file system, namely, mounting
the device does not change file attributes and so can be mounted and seen with
the same attribute no matter where it is mounted, regardless of the mount options.

> [...]

Thanks for the nits and typos pointer. I will fix that.

>> +struct zonefs_super {
>> +
>> +	/* Magic number */
>> +	__le32		s_magic;		/*    4 */
>> +
>> +	/* Metadata version number */
>> +	__le32		s_version;		/*    8 */
>> +
>> +	/* Features */
>> +	__le64		s_features;		/*   16 */
>> +
>> +	/* 128-bit uuid */
>> +	__u8		s_uuid[16];		/*   32 */
>> +
>> +	/* UID/GID to use for files */
>> +	__le32		s_uid;			/*   36 */
>> +	__le32		s_gid;			/*   40 */
>> +
>> +	/* File permissions */
>> +	__le32		s_perm;			/*   44 */
>> +
>> +	/* Padding to 4K */
>> +	__u8		s_reserved[4052];	/* 4096 */
>> +
>> +} __attribute__ ((packed));
> 
> I'm not sure the (end)offset comments are of any value here, it's nothing that
> can't be obtained from pahole or gdb (or even by hand).

OK. Will remove.

>> +/*
>> + * Metadata version.
>> + */
>> +#define ZONEFS_VERSION	1
>> +
>> +/*
>> + * Feature flags.
>> + */
>> +enum zonefs_features {
>> +	/*
>> +	 * Use a zone start sector value as file name.
>> +	 */
>> +	ZONEFS_F_STARTSECT_NAME,
>> +	/*
>> +	 * Aggregate contiguous conventional zones into a single file.
>> +	 */
>> +	ZONEFS_F_AGRCNV,
>> +	/*
>> +	 * Use super block specified UID for files instead of default.
>> +	 */
>> +	ZONEFS_F_UID,
>> +	/*
>> +	 * Use super block specified GID for files instead of default.
>> +	 */
>> +	ZONEFS_F_GID,
>> +	/*
>> +	 * Use super block specified file permissions instead of default 640.
>> +	 */
>> +	ZONEFS_F_PERM,
>> +};
> 
> I'd rather not write the uid, gid, permissions and startsect name to the
> superblock but have it controllable via a mount option. Just write the feature
> to the superblock so we know we _can_ control this per mount.

This is another view. See my thinking above. Thoughts ?

Thanks !
Johannes Thumshirn July 12, 2019, 8:47 a.m. UTC | #3
On Fri, Jul 12, 2019 at 08:31:32AM +0000, Damien Le Moal wrote:
[...]
> > I know I've been advocating for having on-disk metadata, but do we really
> > sacrifice a whole zone per default? I thought we'll have on-disk metadata
> > optional (I might be completely off the track here and need more coffee to
> > wake up though).
> 
> Yes, indeed we do not really need the super block for now. But it is still super
> useful to have so that:
> 1) libblkid and other such userland tools can probe the disk to see its format,
> and preserve the usual "use -force option if you really want to overwrite"
> behavior of all format tools.
> 2) Still related to previous point, the super block allows commands like:
> mount /dev/sdX /mnt
> and
> mount -t zonefs /dev/sdX /mnt
> to have the same result. That is, without the super block, if the drive was
> previously formatted for btrfs or f2fs, the first command will mount that old
> format, while the second will mount zonefs without necessarily erasing the old
> FS super block.
> 3) Having the super block with a version number will allow in the future to add
> more metadata (e.g. file names as decided by the application) while allowing
> backward compatibility of the code.
> 
> >> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
> >> +	for (zone = &zones[1]; zone < end; zone = next) {
> > 
> > [...]
> > 
> >> +
> >> +	/* Set defaults */
> >> +	sbi->s_uid = GLOBAL_ROOT_UID;
> >> +	sbi->s_gid = GLOBAL_ROOT_GID;
> >> +	sbi->s_perm = S_IRUSR | S_IWUSR | S_IRGRP; /* 0640 */
> >> +
> >> +
> >> +	ret = zonefs_read_super(sb);
> >> +	if (ret)
> >> +		return ret;
> > 
> > That would be cool to be controllable via a mount option and have it:
> > 	sbi->s_uid = opt.uid;
> > 	sbi->s_gid = opt.gid;
> > 	sbi->s_perm = opt.mode;
> > 
> > or pass these mount options to zonefs_read_super() and they can be set after
> > the feature validation.
> 
> Yes, I thought of that and even had that implemented in a previous version. I
> switched to the static format time definition only so that the resulting
> operation of the FS is a little more like a normal file system, namely, mounting
> the device does not change file attributes and so can be mounted and seen with
> the same attribute no matter where it is mounted, regardless of the mount options.

[...]

> > I'd rather not write the uid, gid, permissions and startsect name to the
> > superblock but have it controllable via a mount option. Just write the feature
> > to the superblock so we know we _can_ control this per mount.
> 
> This is another view. See my thinking above. Thoughts ?

Hm, both a valid views and I'm not sure which is better for the production use
cases either.

With the approach I had in mind one could pre-format dozens of drives and
deploy them in the field. The admins then can decide what
UID/GID/Permission/etc.. the application layer needs for a particular drive
and supply these parameters on mount time.

With the approach you implemented here we don't have the surprises if someone
accidentally (or maliciously) passed the wrong parameters.

A combined approach is also not 100% discussion free, as what has preference,
on-disk or mount time.

I'll be thinking about it and come back once I have an idea.

Byte,
	Johannes
Viacheslav Dubeyko July 12, 2019, 5:10 p.m. UTC | #4
On Fri, 2019-07-12 at 12:00 +0900, Damien Le Moal wrote:
> zonefs is a very simple file system exposing each zone of a zoned
> block device as a file. This is intended to simplify implementation 

As far as I can see, a zone usually is pretty big in size (for example,
256MB). But [1, 2] showed that about 60% of files on a file system
volume has size about 4KB - 128KB. Also [3] showed that modern
application uses a very complex files' structures that are updated in
random order. Moreover, [4] showed that 90% of all files are not used
after initial creation, those that are used are normally short-lived,
and that if a file is not used in some manner the day after it is
created, it will probably never be used; 1% of all files are used
daily.

It sounds for me that mostly this approach will lead to waste of zones'
space. Also, the necessity to update data of the same file will be
resulted in frequent moving of files' data from one zone to another
one. If we are talking about SSDs then it sounds like quick and easy
way to kill this device fast.

Do you have in mind some special use-case?

Thanks,
Viacheslav Dubeyko.


[1] Agrawal, et al., “A Five-Year Study of File-System Metadata,” ACM
Transactions on Storage (TOS), vol. 3 Issue 3, Oct. 2007, Article No.
9.
[2] Douceur, et al., “A Large-Scale Study of File-System Contents,”
SIGMETRICS '99 Proceedings of the 1999 ACM SIGMETRICS international
conference on Measurement and modeling of computer systems, pp. 59-70,
May 1-4, 1999.
[3] Tyler Harter, Chris Dragga, Michael Vaughn, Andrea C. Arpaci-
Dusseau, and Remzi H. Arpaci-Dusseau, “A file is not a file:
understanding the I/O behavior of Apple desktop applications.” In
Proceedings of the Twenty-Third ACM Symposium on Operating Systems
Principles (SOSP '11). ACM, New York, NY, USA, 71-83.
[4] Tim Gibson, Ethan L. Miller, Darrell D. E. Long, “Long-term File
Activity and Inter-Reference Patterns.”
Damien Le Moal July 12, 2019, 10:56 p.m. UTC | #5
On 2019/07/13 2:10, Viacheslav Dubeyko wrote:
> On Fri, 2019-07-12 at 12:00 +0900, Damien Le Moal wrote:
>> zonefs is a very simple file system exposing each zone of a zoned
>> block device as a file. This is intended to simplify implementation 
> 
> As far as I can see, a zone usually is pretty big in size (for example,
> 256MB). But [1, 2] showed that about 60% of files on a file system
> volume has size about 4KB - 128KB. Also [3] showed that modern
> application uses a very complex files' structures that are updated in
> random order. Moreover, [4] showed that 90% of all files are not used
> after initial creation, those that are used are normally short-lived,
> and that if a file is not used in some manner the day after it is
> created, it will probably never be used; 1% of all files are used
> daily.
> 
> It sounds for me that mostly this approach will lead to waste of zones'
> space. Also, the necessity to update data of the same file will be
> resulted in frequent moving of files' data from one zone to another
> one. If we are talking about SSDs then it sounds like quick and easy
> way to kill this device fast.
> 
> Do you have in mind some special use-case?

As the commit message mentions, zonefs is not a traditional file system by any
mean and much closer to a raw block device access interface than anything else.
This is the entire point of this exercise: allow replacing the raw block device
accesses with the easier to use file system API. Raw block device access is also
file API so one could argue that this is nonsense. What I mean here is that by
abstracting zones with files, the user does not need to do the zone
configuration discovery with ioctl(BLKREPORTZONES), does not need to do explicit
zone resets with ioctl(BLKRESETZONE), does not have to "start from one sector
and write sequentially from there" management for write() calls (i.e. seeks),
etc. This is all replaced with the file abstraction: directory entry list
replace zone information, truncate() replace zone reset, file current position
replaces the application zone write pointer management.

This simplifies implementing support of applications for zoned block devices,
but only in cases where said applications:
1) Operate with large files
2) have no or only minimal need for random writes

A perfect match for this as mentioned in the commit message are LSM-tree based
applications such as LevelDB or RocksDB. Other examples, related, include
Bluestore distributed object store which uses RocksDB but still has a bluefs
layer that could be replaced with zonefs.

As an illustration of this, Ting Yao of Huazhong University of Science and
Technology (China) and her team modified LevelDB to work with zonefs. The early
prototype code is on github here: https://github.com/PDS-Lab/GearDB/tree/zonefs

LSM-Tree applications typically operate on large files, in the same range as
zoned block device zone size (e.g. 256 MB or so). While this is generally a
parameter that can be changed, the use of zonefs and zoned block device forces
using the zone size as the SSTable file maximum size. This can have an impact on
the DB performance depending on the device type, but that is another discussion.
The point here is the code simplifications that zonefs allows.

For more general purpose use cases (small files, lots of random modifications),
we already have the dm-zoned device mapper and f2fs support and we are also
currently working on btrfs support. These solutions are in my opinion more
appropriate than zonefs to address the points you raised.

Best regards.
Dave Chinner July 15, 2019, 1:19 a.m. UTC | #6
Just a few quick things as I read through this to see how it uses
iomap....

On Fri, Jul 12, 2019 at 12:00:17PM +0900, Damien Le Moal wrote:
> +static int zonefs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> +			      unsigned int flags, struct iomap *iomap)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	loff_t max_isize = zonefs_file_max_size(inode);
> +	loff_t isize = i_size_read(inode);
> +
> +	/*
> +	 * For sequential zones, enforce direct IO writes. This is already
> +	 * checked when writes are issued, so warn about this here if we
> +	 * get buffered write to a sequential file inode.
> +	 */
> +	if (WARN_ON_ONCE(zonefs_file_is_seq(inode) && (flags & IOMAP_WRITE) &&
> +			 (!(flags & IOMAP_DIRECT))))
                         ^ Excess (..).

> +		return -EIO;

> +	/* An IO cannot exceed the zone size */
> +	if (offset >= max_isize)
> +		return -EFBIG;

So a write() call that is for a length longer than max_isize is
going to end up being a short write? i.e. iomap_apply() will loop
mapping the inode until either we reach the end of the user write
or we hit max_isize?

How is userspace supposed to tell the difference between a short
write and a write that overruns max_isize?

> +	/* All blocks are always mapped */
> +	if (offset >= i_size_read(inode)) {
> +		length = min(length, max_isize - offset);
> +		iomap->type = IOMAP_UNWRITTEN;
> +	} else {
> +		length = min(length, isize - offset);
> +		iomap->type = IOMAP_MAPPED;
> +	}
> +	iomap->offset = offset & (~sbi->s_blocksize_mask);
> +	iomap->length = (offset + length + sbi->s_blocksize_mask) &
> +			(~sbi->s_blocksize_mask);
> +	iomap->addr = zonefs_file_addr(inode) + iomap->offset;
> +	iomap->bdev = inode->i_sb->s_bdev;
> +
> +	return 0;
> +}
> +
> +static const struct iomap_ops zonefs_iomap_ops = {
> +	.iomap_begin	= zonefs_iomap_begin,
> +};
> +
> +static int zonefs_readpage(struct file *unused, struct page *page)
> +{
> +	return iomap_readpage(page, &zonefs_iomap_ops);
> +}
> +
> +static int zonefs_readpages(struct file *unused, struct address_space *mapping,
> +			    struct list_head *pages, unsigned int nr_pages)
> +{
> +	return iomap_readpages(mapping, pages, nr_pages, &zonefs_iomap_ops);
> +}
> +
> +static int zonefs_map_blocks(struct iomap_writepage_ctx *wpc,
> +			     struct inode *inode, loff_t offset)
> +{
> +	if (offset >= wpc->iomap.offset &&
> +	    offset < wpc->iomap.offset + wpc->iomap.length)
> +		return 0;
> +
> +	memset(&wpc->iomap, 0, sizeof(wpc->iomap));
> +	return zonefs_iomap_begin(inode, offset, INT_MAX, 0, &wpc->iomap);

Why is the write length set to INT_MAX here? What happens when we
get a zone that is larger than 2GB? i.e. the length parameter is a
loff_t, not an int....


> +static int zonefs_truncate_seqfile(struct inode *inode)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	int ret;
> +
> +	/* Serialize against page faults */
> +	down_write(&zi->i_mmap_sem);
> +
> +	ret = blkdev_reset_zones(inode->i_sb->s_bdev,
> +				 zonefs_file_addr(inode) >> SECTOR_SHIFT,
> +				 zonefs_file_max_size(inode) >> SECTOR_SHIFT,
> +				 GFP_KERNEL);

Not sure GFP_KERNEL is safe here. This is called holding a
filesystem lock here, so it's not immediately clear to me if this
can deadlock through memory reclaim or not...

> +	if (ret) {
> +		zonefs_err(inode->i_sb,
> +			   "zonefs: Reset zone at %llu failed %d",
> +			   zonefs_file_addr(inode) >> SECTOR_SHIFT,
> +			   ret);

redundant "zonefs" in error message.

> +	} else {
> +		truncate_setsize(inode, 0);
> +		zi->i_wpoffset = 0;
> +	}
> +
> +	up_write(&zi->i_mmap_sem);
> +
> +	return ret;
> +}
> +
> +static int zonefs_inode_setattr(struct dentry *dentry, struct iattr *iattr)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	int ret;
> +
> +	ret = setattr_prepare(dentry, iattr);
> +	if (ret)
> +		return ret;
> +
> +	if ((iattr->ia_valid & ATTR_UID &&
> +	     !uid_eq(iattr->ia_uid, inode->i_uid)) ||
> +	    (iattr->ia_valid & ATTR_GID &&
> +	     !gid_eq(iattr->ia_gid, inode->i_gid))) {
> +		ret = dquot_transfer(inode, iattr);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (iattr->ia_valid & ATTR_SIZE) {
> +		/* The size of conventional zone files cannot be changed */
> +		if (zonefs_file_is_conv(inode))
> +			return -EPERM;
> +
> +		/*
> +		 * For sequential zone files, we can only allow truncating to
> +		 * 0 size which is equivalent to a zone reset.
> +		 */
> +		if (iattr->ia_size != 0)
> +			return -EPERM;
> +
> +		ret = zonefs_truncate_seqfile(inode);
> +		if (ret)
> +			return ret;

Ok, so we are calling zonefs_truncate_seqfile() holding the i_rwsem
as well. That does tend to imply GFP_NOFS should probably be used
for the blkdev_reset_zones() call.

> +	}
> +
> +	setattr_copy(inode, iattr);
> +
> +	return 0;
> +}
> +
> +static const struct inode_operations zonefs_file_inode_operations = {
> +	.setattr	= zonefs_inode_setattr,
> +};
> +
> +/*
> + * Open a file.
> + */
> +static int zonefs_file_open(struct inode *inode, struct file *file)
> +{
> +	/*
> +	 * Note: here we can do an explicit open of the file zone,
> +	 * on the first open of the inode. The explicit close can be
> +	 * done on the last release (close) call for the inode.
> +	 */
> +
> +	return generic_file_open(inode, file);
> +}

Why is a wrapper needed for this?

> +static int zonefs_file_fsync(struct file *file, loff_t start, loff_t end,
> +			     int datasync)
> +{
> +	struct inode *inode = file_inode(file);
> +	int ret;
> +
> +	/*
> +	 * Since only direct writes are allowed in sequential files, we only
> +	 * need a device flush for these files.
> +	 */
> +	if (zonefs_file_is_seq(inode))
> +		goto flush;
> +
> +	ret = file_write_and_wait_range(file, start, end);
> +	if (ret == 0)
> +		ret = file_check_and_advance_wb_err(file);
> +	if (ret)
> +		return ret;

> +
> +flush:
> +	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);

The goto can be avoided in this case simply:

	if (zonefs_file_is_conv(inode)) {
		/* do flush */
	}
	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);

> +}
> +
> +static vm_fault_t zonefs_filemap_fault(struct vm_fault *vmf)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(file_inode(vmf->vma->vm_file));
> +	vm_fault_t ret;
> +
> +	down_read(&zi->i_mmap_sem);
> +	ret = filemap_fault(vmf);
> +	up_read(&zi->i_mmap_sem);
> +
> +	return ret;
> +}
> +
> +static vm_fault_t zonefs_filemap_page_mkwrite(struct vm_fault *vmf)
> +{
> +	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	vm_fault_t ret;
> +
> +	sb_start_pagefault(inode->i_sb);
> +	file_update_time(vmf->vma->vm_file);
> +
> +	/* Serialize against truncates */
> +	down_read(&zi->i_mmap_sem);
> +	ret = iomap_page_mkwrite(vmf, &zonefs_iomap_ops);
> +	up_read(&zi->i_mmap_sem);
> +
> +	sb_end_pagefault(inode->i_sb);
> +	return ret;
> +}
> +
> +static const struct vm_operations_struct zonefs_file_vm_ops = {
> +	.fault		= zonefs_filemap_fault,
> +	.map_pages	= filemap_map_pages,
> +	.page_mkwrite	= zonefs_filemap_page_mkwrite,
> +};
> +
> +static int zonefs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	/*
> +	 * Conventional zone files can be mmap-ed READ/WRITE.
> +	 * For sequential zone files, only readonly mappings are possible.
> +	 */
> +	if (zonefs_file_is_seq(file_inode(file)) &&
> +	    (vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> +		return -EINVAL;
> +
> +	file_accessed(file);
> +	vma->vm_ops = &zonefs_file_vm_ops;
> +
> +	return 0;
> +}
> +
> +static loff_t zonefs_file_llseek(struct file *file, loff_t offset, int whence)
> +{
> +	loff_t isize = i_size_read(file_inode(file));
> +
> +	/*
> +	 * Seeks are limited to below the zone size for conventional zones
> +	 * and below the zone write pointer for sequential zones. In both
> +	 * cases, this limit is the inode size.
> +	 */
> +	return generic_file_llseek_size(file, offset, whence, isize, isize);
> +}
> +
> +static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	loff_t max_pos = zonefs_file_max_size(inode);
> +	size_t count;
> +	ssize_t ret = 0;
> +
> +	/*
> +	 * Check that the read operation does not go beyond the maximum
> +	 * file size.
> +	 */
> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
> +		return -EFBIG;
> +
> +	/*
> +	 * For sequential zones, limit reads to written data.
> +	 */
> +	if (zonefs_file_is_seq(inode))
> +		max_pos = i_size_read(inode);
> +	if (iocb->ki_pos >= max_pos)
> +		return 0;

Isn't this true for both types of zone at this point? i.e. at this
point:

	max_pos = i_size_read(inode);
	if (iocb->ki_pos >= max_pos)
		return 0;

because i_size is either the zonefs_file_max_size() for conventional
zones (which we've already checked) or it's the write pointer for
a sequential zone. i.e. it's the max position for either case.

> +	iov_iter_truncate(to, max_pos - iocb->ki_pos);
> +	count = iov_iter_count(to);
> +	if (!count)
> +		return 0;

The iov_iter should never be zero length here, because that implies
the position was >= max_pos and that will be caught by the above
checks...

> +	/* Direct IO reads must be aligned to device physical sector size */
> +	if ((iocb->ki_flags & IOCB_DIRECT) &&
> +	    ((iocb->ki_pos | count) & sbi->s_blocksize_mask))
> +		return -EINVAL;
> +
> +	if (iocb->ki_flags & IOCB_NOWAIT) {
> +		if (!inode_trylock_shared(inode))
> +			return -EAGAIN;
> +	} else {
> +		inode_lock_shared(inode);
> +	}

IIUC, write IO completion takes the inode lock to serialise file
size updates for sequential zones. In that case, shouldn't this lock
be taken before we do the EOF checks above?

> +	if (iocb->ki_flags & IOCB_DIRECT) {
> +		file_accessed(iocb->ki_filp);
> +		ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, NULL);
> +	} else {
> +		ret = generic_file_read_iter(iocb, to);
> +	}
> +
> +	inode_unlock_shared(inode);
> +
> +	return ret;
> +}
> +
> +/*
> + * We got a write error: get the sequenial zone information from the device to
> + * figure out where the zone write pointer is and verify the inode size against
> + * it.
> + */
> +static int zonefs_write_failed(struct inode *inode, int error)
> +{
> +	struct super_block *sb = inode->i_sb;
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	sector_t sector = zi->i_addr >> SECTOR_SHIFT;
> +	unsigned int noio_flag;
> +	struct blk_zone zone;
> +	int n = 1, ret;
> +
> +	zonefs_warn(sb, "Updating inode zone %llu info\n", sector);
> +
> +	noio_flag = memalloc_noio_save();
> +	ret = blkdev_report_zones(sb->s_bdev, sector, &zone, &n);
> +	memalloc_noio_restore(noio_flag);

What deadlock does the memalloc_noio_save() avoid? There should be a
comment explaining what problem memalloc_noio_save() avoids
everywhere it is used like this. If it isn't safe to do GFP_KERNEL
allocations here under the i_rwsem, then why would it be safe to
do GFP_KERNEL allocations in the truncate code under the i_rwsem?

> +
> +	if (!n)
> +		ret = -EIO;
> +	if (ret) {
> +		zonefs_err(sb, "Get zone %llu report failed %d\n",
> +			   sector, ret);
> +		return ret;
> +	}
> +
> +	zi->i_wpoffset = (zone.wp - zone.start) << SECTOR_SHIFT;
> +	if (i_size_read(inode) != zi->i_wpoffset) {
> +		i_size_write(inode, zi->i_wpoffset);
> +		truncate_pagecache(inode, zi->i_wpoffset);
> +	}

This looks .... dangerous. If the write pointer was advanced, but
the data wasn't written properly, this causes stale data exposure on
write failure. i.e. it's not failsafe.

I suspect that on a sequential zone write failure and the write
pointer does not equal the offset of the write, we should consider
the zone corrupt. Also, this is for direct IO completion for
sequential writes, yes? So what does the page cache truncation
acheive given that only direct IO writes are allowed to these files?

> +
> +	return error;
> +}
> +
> +static int zonefs_update_size(struct inode *inode, loff_t new_pos)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +
> +	zi->i_wpoffset = new_pos;
> +	if (new_pos > i_size_read(inode))
> +		i_size_write(inode, new_pos);
> +	return 0;
> +}
> +
> +static int zonefs_dio_seqwrite_end_io(struct kiocb *iocb, ssize_t size,
> +				      unsigned int flags)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	int ret;
> +
> +	inode_lock(inode);
> +	if (size < 0)
> +		ret = zonefs_write_failed(inode, size);
> +	else
> +		ret = zonefs_update_size(inode, iocb->ki_pos + size);
> +	inode_unlock(inode);
> +	return ret;

Shouldn't this have a check that it's being called on a sequential
zone inode?

> +}
> +
> +static ssize_t zonefs_file_dio_aio_write(struct kiocb *iocb,
> +					 struct iov_iter *from)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	size_t count;
> +
> +	/*
> +	 * The size of conventional zone files is fixed to the zone size.
> +	 * So only direct writes to sequential zones need adjusting the
> +	 * inode size on IO completion.
> +	 */
> +	if (zonefs_file_is_conv(inode))
> +		return iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
> +
> +	/* Enforce append only sequential writes */
> +	count = iov_iter_count(from);
> +	if (iocb->ki_pos != zi->i_wpoffset) {
> +		zonefs_err(inode->i_sb,
> +			   "Unaligned write at %llu + %zu (wp %llu)\n",
> +			   iocb->ki_pos, count, zi->i_wpoffset);
> +		return -EINVAL;
> +	}
> +
> +	if (is_sync_kiocb(iocb)) {
> +		/*
> +		 * Don't use the end_io callback for synchronous iocbs,
> +		 * as we'd deadlock on i_rwsem.  Instead perform the same
> +		 * actions manually here.
> +		 */
> +		count = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
> +		if (count < 0)
> +			return zonefs_write_failed(inode, count);
> +		zonefs_update_size(inode, iocb->ki_pos);
> +		return count;

Urk. This locking is nasty, and doesn't avoid the problem.....

> +	}
> +
> +	return iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
> +			    zonefs_dio_seqwrite_end_io);

... because I think this can deadlock.

AFAIA, the rule is that IO completion callbacks cannot take
a lock that is held across IO submission. The reason is that
IO can complete so fast that the submission code runs the
completion. i.e. iomap_dio_rw() can be the function that calls
iomap_dio_complete() and runs the IO completion.

In which case, this will deadlock because we are already holding the
i_rwsem and the end_io completion will try to take it again.



> +}
> +
> +static ssize_t zonefs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	size_t count;
> +	ssize_t ret;
> +
> +	/*
> +	 * Check that the read operation does not go beyond the file
> +	 * zone boundary.
> +	 */
> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
> +		return -EFBIG;
> +	iov_iter_truncate(from, zonefs_file_max_size(inode) - iocb->ki_pos);
> +	count = iov_iter_count(from);
> +
> +	if (!count)
> +		return 0;
> +
> +	/*
> +	 * Direct IO writes are mandatory for sequential zones so that write IO
> +	 * order is preserved. The direct writes also must be aligned to
> +	 * device physical sector size.
> +	 */
> +	if (iocb->ki_flags & IOCB_DIRECT) {
> +		if ((iocb->ki_pos | count) & sbi->s_blocksize_mask)
> +			return -EINVAL;
> +	} else {
> +		if (zonefs_file_is_seq(inode))
> +			return -EOPNOTSUPP;

zonefs_iomap_begin() returns -EIO in this case and issues a warning.
This seems somewhat inconsistent....

> +	}
> +
> +	if (iocb->ki_flags & IOCB_NOWAIT) {
> +		if (!inode_trylock(inode))
> +			return -EAGAIN;
> +	} else {
> +		inode_lock(inode);
> +	}
> +
> +	ret = generic_write_checks(iocb, from);
> +	if (ret <= 0)
> +		goto out;

Shouldn't this be done before the iov_iter is truncated?

> +
> +	if (iocb->ki_flags & IOCB_DIRECT)
> +		ret = zonefs_file_dio_aio_write(iocb, from);
> +	else
> +		ret = iomap_file_buffered_write(iocb, from, &zonefs_iomap_ops);
> +
> +out:
> +	inode_unlock(inode);
> +
> +	if (ret > 0 && (!(iocb->ki_flags & IOCB_DIRECT))) {
> +		iocb->ki_pos += ret;
> +		ret = generic_write_sync(iocb, ret);
> +	}

Hmmm. The split of checks and doing stuff between direct IO and
buffered IO seems a bit arbitrary. e.g. the "sequential zones can
only do append writes" is in zonefs_file_dio_aio_write(), but we
do a check that "sequential zones can only do direct IO" here.

And then we have the sync code that can only occur on buffered IO,
which we don't have a wrapper function for but really should.  And I
suspect that the locking is going to have to change here because of
the direct IO issues, so maybe it would be best to split this up
similar to the way XFS has two completely separate functions for the
two paths....


> +static struct kmem_cache *zonefs_inode_cachep;
> +
> +static struct inode *zonefs_alloc_inode(struct super_block *sb)
> +{
> +	struct zonefs_inode_info *zi;
> +
> +	zi = kmem_cache_alloc(zonefs_inode_cachep, GFP_KERNEL);
> +	if (!zi)
> +		return NULL;
> +
> +	init_rwsem(&zi->i_mmap_sem);
> +	inode_init_once(&zi->i_vnode);
> +
> +	return &zi->i_vnode;
> +}
> +
> +static void zonefs_destroy_cb(struct rcu_head *head)
> +{
> +	struct inode *inode = container_of(head, struct inode, i_rcu);
> +
> +	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
> +}
> +
> +static void zonefs_destroy_inode(struct inode *inode)
> +{
> +	call_rcu(&inode->i_rcu, zonefs_destroy_cb);
> +}

If this is all the inode destructor is, then implement ->free_inode
instead. i.e.

zonefs_free_inode(inode)
{
	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
}

and the VFS takes care of the RCU freeing of the inode.

> +/*
> + * File system stat.
> + */
> +static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
> +{
> +	struct super_block *sb = dentry->d_sb;
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	sector_t nr_sectors = sb->s_bdev->bd_part->nr_sects;
> +	enum zonefs_ztype t;
> +
> +	buf->f_type = ZONEFS_MAGIC;
> +	buf->f_bsize = dentry->d_sb->s_blocksize;
> +	buf->f_namelen = ZONEFS_NAME_MAX;
> +
> +	buf->f_blocks = nr_sectors >> (sb->s_blocksize_bits - SECTOR_SHIFT);
> +	buf->f_bfree = 0;
> +	buf->f_bavail = 0;
> +
> +	buf->f_files = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - 1;
> +	for (t = ZONEFS_ZTYPE_ALL; t < ZONEFS_ZTYPE_MAX; t++) {
> +		if (sbi->s_nr_zones[t])
> +			buf->f_files++;
> +	}
> +	buf->f_ffree = 0;
> +
> +	/* buf->f_fsid = 0; uuid, see ext2 */

This doesn't tell me anything useful. Does it mean "we should use
the uuid like ext2" or something else? is it a "TODO:" item?

> +	buf->f_namelen = ZONEFS_NAME_MAX;

You've done this twice. :)

> +static char *zgroups_name[ZONEFS_ZTYPE_MAX] = {
> +	NULL,
> +	"cnv",
> +	"seq"
> +};

What's the reason for a NULL in the first entry?

> +
> +/*
> + * Create a zone group and populate it with zone files.
> + */
> +static int zonefs_create_zgroup(struct super_block *sb, struct blk_zone *zones,
> +				enum zonefs_ztype type)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	struct blk_zone *zone, *next, *end;
> +	char name[ZONEFS_NAME_MAX];
> +	unsigned int nr_files = 0;
> +	struct dentry *dir;
> +
> +	/* If the group is empty, nothing to do */
> +	if (!sbi->s_nr_zones[type])
> +		return 0;
> +
> +	dir = zonefs_create_inode(sb->s_root, zgroups_name[type], NULL);
> +	if (!dir)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Note: The first zone contains the super block: skip it.
> +	 */
> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
> +	for (zone = &zones[1]; zone < end; zone = next) {
> +
> +		next = zone + 1;
> +		if (zonefs_zone_type(zone) != type)
> +			continue;
> +
> +		/* Ignore offline zones */
> +		if (zonefs_zone_offline(zone))
> +			continue;
> +
> +		/*
> +		 * For conventional zones, contiguous zones can be aggregated
> +		 * together to form larger files.
> +		 * Note that this overwrites the length of the first zone of
> +		 * the set of contiguous zones aggregated together.
> +		 * Only zones with the same condition can be agreggated so that
> +		 * offline zones are excluded and readonly zones are aggregated
> +		 * together into a read only file.
> +		 */
> +		if (type == ZONEFS_ZTYPE_CNV &&
> +		    zonefs_has_feature(sbi, ZONEFS_F_AGRCNV)) {
> +			for (; next < end; next++) {
> +				if (zonefs_zone_type(next) != type ||
> +				    next->cond != zone->cond)
> +					break;
> +				zone->len += next->len;
> +			}
> +		}
> +
> +		if (zonefs_has_feature(sbi, ZONEFS_F_STARTSECT_NAME))
> +			/* Use zone start sector as file names */
> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%llu",
> +				 zone->start);
> +		else
> +			/* Use file number as file names */
> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%u", nr_files);
> +		nr_files++;
> +
> +		if (!zonefs_create_inode(dir, name, zone))
> +			return -ENOMEM;

I guess this means partial setup due to failure needs to be torn
down by the kill_super() code?

> +	}
> +
> +	zonefs_info(sb, "Zone group %d (%s), %u zones -> %u file%s\n",
> +		    type, zgroups_name[type], sbi->s_nr_zones[type],
> +		    nr_files, nr_files > 1 ? "s" : "");
> +
> +	return 0;
> +}
> +
> +static struct blk_zone *zonefs_get_zone_info(struct super_block *sb)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	struct block_device *bdev = sb->s_bdev;
> +	sector_t nr_sectors = bdev->bd_part->nr_sects;
> +	unsigned int i, n, nr_zones = 0;
> +	struct blk_zone *zones, *zone;
> +	sector_t sector = 0;
> +	int ret;
> +
> +	sbi->s_blocksize_mask = sb->s_blocksize - 1;
> +	sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] = blkdev_nr_zones(bdev);
> +	zones = kvcalloc(sbi->s_nr_zones[ZONEFS_ZTYPE_ALL],
> +			 sizeof(struct blk_zone), GFP_KERNEL);
> +	if (!zones)
> +		return ERR_PTR(-ENOMEM);

Hmmm. That's a big allocation. That might be several megabytes for a
typical 16TB SMR drive, right? It might be worth adding a comment
indicating just how large this is, because it's somewhat unusual in
kernel space, even for temporary storage.

> --- /dev/null
> +++ b/fs/zonefs/zonefs.h
> @@ -0,0 +1,190 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple zone file system for zoned block devices.
> + *
> + * Copyright (C) 2019 Western Digital Corporation or its affiliates.
> + */
> +#ifndef __ZONEFS_H__
> +#define __ZONEFS_H__
> +
> +#include <linux/fs.h>
> +#include <linux/magic.h>
> +
> +/*
> + * Maximum length of file names: this only needs to be large enough to fit
> + * the zone group directory names and a decimal value of the start sector of
> + * the zones for file names. 16 characterse is plenty.
> + */
> +#define ZONEFS_NAME_MAX		16
> +
> +/*
> + * Zone types: ZONEFS_ZTYPE_SEQWRITE is used for all sequential zone types

ZONEFS_ZTYPE_SEQ?

> + * defined in linux/blkzoned.h, that is, BLK_ZONE_TYPE_SEQWRITE_REQ and
> + * BLK_ZONE_TYPE_SEQWRITE_PREF.
> + */
> +enum zonefs_ztype {
> +	ZONEFS_ZTYPE_ALL = 0,
> +	ZONEFS_ZTYPE_CNV,
> +	ZONEFS_ZTYPE_SEQ,
> +	ZONEFS_ZTYPE_MAX,
> +};

What is ZONEFS_ZTYPE_ALL supposed to be used for?

> +static inline bool zonefs_zone_offline(struct blk_zone *zone)
> +{
> +	return zone->cond == BLK_ZONE_COND_OFFLINE;
> +}
> +
> +static inline bool zonefs_zone_readonly(struct blk_zone *zone)
> +{
> +	return zone->cond == BLK_ZONE_COND_READONLY;
> +}

These should be block layer helpers as the operate on blk_zone,
not zonefs structures.

> +
> +/*
> + * Inode private data.
> + */
> +struct zonefs_inode_info {
> +	struct inode		i_vnode;
> +	enum zonefs_ztype	i_ztype;
> +	loff_t			i_addr;
> +	loff_t			i_wpoffset;
> +	loff_t			i_max_size;
> +	struct rw_semaphore	i_mmap_sem;
> +};
> +
> +static inline struct zonefs_inode_info *ZONEFS_I(struct inode *inode)
> +{
> +	return container_of(inode, struct zonefs_inode_info, i_vnode);
> +}
> +
> +static inline bool zonefs_file_is_conv(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_CNV;
> +}
> +
> +static inline bool zonefs_file_is_seq(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_SEQ;
> +}
> +
> +/*
> + * Address (byte offset) on disk of a file zone.
> + */
> +static inline loff_t zonefs_file_addr(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_addr;
> +}

so it's a disk address, but it's encoded in bytes rather than sectors
so that makes it an offset. That's kinda confusing coming from a
filesystem that makes a clear distinction between these two things.

> +
> +/*
> + * Maximum possible size of a file (i.e. the zone size).
> + */
> +static inline loff_t zonefs_file_max_size(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_max_size;
> +}
> +
> +/*
> + * On-disk super block (block 0).
> + */
> +struct zonefs_super {
> +
> +	/* Magic number */
> +	__le32		s_magic;		/*    4 */
> +
> +	/* Metadata version number */
> +	__le32		s_version;		/*    8 */
> +
> +	/* Features */
> +	__le64		s_features;		/*   16 */

On-disk version numbers are kinda redundant when you have
fine grained feature fields to indicate the on-disk layout...

> +/*
> + * Feature flags.
> + */
> +enum zonefs_features {
> +	/*
> +	 * Use a zone start sector value as file name.
> +	 */
> +	ZONEFS_F_STARTSECT_NAME,
> +	/*
> +	 * Aggregate contiguous conventional zones into a single file.
> +	 */
> +	ZONEFS_F_AGRCNV,
> +	/*
> +	 * Use super block specified UID for files instead of default.
> +	 */
> +	ZONEFS_F_UID,
> +	/*
> +	 * Use super block specified GID for files instead of default.
> +	 */
> +	ZONEFS_F_GID,
> +	/*
> +	 * Use super block specified file permissions instead of default 640.
> +	 */
> +	ZONEFS_F_PERM,
> +};

Are these the on-disk feature bit definitions, or just used in
memory? Or both?

Cheers,

Dave.
Johannes Thumshirn July 15, 2019, 6:57 a.m. UTC | #7
On Mon, Jul 15, 2019 at 11:19:35AM +1000, Dave Chinner wrote:
[...]
> > +/*
> > + * Open a file.
> > + */
> > +static int zonefs_file_open(struct inode *inode, struct file *file)
> > +{
> > +	/*
> > +	 * Note: here we can do an explicit open of the file zone,
> > +	 * on the first open of the inode. The explicit close can be
> > +	 * done on the last release (close) call for the inode.
> > +	 */
> > +
> > +	return generic_file_open(inode, file);
> > +}
> 
> Why is a wrapper needed for this?

AFAIR this is a left over from an older patch of me where an open of a
sequential only zone automagically appended O_APPEND, but this broke all kinds
of assumptions user-space did so Damien ripped it out again. So yes the
wrapper can go as well.

Byte,
	Johannes
Viacheslav Dubeyko July 15, 2019, 4:54 p.m. UTC | #8
On Fri, 2019-07-12 at 22:56 +0000, Damien Le Moal wrote:
> On 2019/07/13 2:10, Viacheslav Dubeyko wrote:
> > 
> > On Fri, 2019-07-12 at 12:00 +0900, Damien Le Moal wrote:
> > > 
> > > zonefs is a very simple file system exposing each zone of a zoned
> > > block device as a file. This is intended to simplify
> > > implementation 
> > As far as I can see, a zone usually is pretty big in size (for
> > example,
> > 256MB). But [1, 2] showed that about 60% of files on a file system
> > volume has size about 4KB - 128KB. Also [3] showed that modern
> > application uses a very complex files' structures that are updated
> > in
> > random order. Moreover, [4] showed that 90% of all files are not
> > used
> > after initial creation, those that are used are normally short-
> > lived,
> > and that if a file is not used in some manner the day after it is
> > created, it will probably never be used; 1% of all files are used
> > daily.
> > 
> > It sounds for me that mostly this approach will lead to waste of
> > zones'
> > space. Also, the necessity to update data of the same file will be
> > resulted in frequent moving of files' data from one zone to another
> > one. If we are talking about SSDs then it sounds like quick and
> > easy
> > way to kill this device fast.
> > 
> > Do you have in mind some special use-case?
> As the commit message mentions, zonefs is not a traditional file
> system by any
> mean and much closer to a raw block device access interface than
> anything else.
> This is the entire point of this exercise: allow replacing the raw
> block device
> accesses with the easier to use file system API. Raw block device
> access is also
> file API so one could argue that this is nonsense. What I mean here
> is that by
> abstracting zones with files, the user does not need to do the zone
> configuration discovery with ioctl(BLKREPORTZONES), does not need to
> do explicit
> zone resets with ioctl(BLKRESETZONE), does not have to "start from
> one sector
> and write sequentially from there" management for write() calls (i.e.
> seeks),
> etc. This is all replaced with the file abstraction: directory entry
> list
> replace zone information, truncate() replace zone reset, file current
> position
> replaces the application zone write pointer management.
> 
> This simplifies implementing support of applications for zoned block
> devices,
> but only in cases where said applications:
> 1) Operate with large files
> 2) have no or only minimal need for random writes
> 
> A perfect match for this as mentioned in the commit message are LSM-
> tree based
> applications such as LevelDB or RocksDB. Other examples, related,
> include
> Bluestore distributed object store which uses RocksDB but still has a
> bluefs
> layer that could be replaced with zonefs.
> 
> As an illustration of this, Ting Yao of Huazhong University of
> Science and
> Technology (China) and her team modified LevelDB to work with zonefs.
> The early
> prototype code is on github here: https://github.com/PDS-Lab/GearDB/t
> ree/zonefs
> 
> LSM-Tree applications typically operate on large files, in the same
> range as
> zoned block device zone size (e.g. 256 MB or so). While this is
> generally a
> parameter that can be changed, the use of zonefs and zoned block
> device forces
> using the zone size as the SSTable file maximum size. This can have
> an impact on
> the DB performance depending on the device type, but that is another
> discussion.
> The point here is the code simplifications that zonefs allows.
> 
> For more general purpose use cases (small files, lots of random
> modifications),
> we already have the dm-zoned device mapper and f2fs support and we
> are also
> currently working on btrfs support. These solutions are in my opinion
> more
> appropriate than zonefs to address the points you raised.
> 

Sounds pretty reasonable. But I still have two worries.

First of all, even modest file system could contain about 100K files on
a volume. So, if our zone is 256 MB then we need in 24 TB storage
device for 100K files. Even if we consider some special use-case of
database, for example, then it's pretty easy to imagine the creation a
lot of files. So, are we ready to provide such huge storage devices
(especially, for the case of SSDs)?

Secondly, the allocation scheme is too simplified for my taste and it
could create a significant fragmentation of a volume. Again, 256 MB is
pretty big size. So, I assume that, mostly, it will be allocated only
one zone at first for a created file. If file grows then it means that
it will need to allocate the two contigous zones and to move the file's
content. Finally, it sounds for me that it is possible to create a lot
of holes and to achieve the volume state when it exists a lot of free
space but files will be unable to grow and it will be impossible to add
a new data on the volume. Have you made an estimation of the suggested
allocation scheme?

Thanks,
Viacheslav Dubeyko.
Damien Le Moal July 15, 2019, 11:53 p.m. UTC | #9
On 2019/07/16 1:54, Viacheslav Dubeyko wrote:
[...]
>>> Do you have in mind some special use-case?
>> As the commit message mentions, zonefs is not a traditional file
>> system by any
>> mean and much closer to a raw block device access interface than
>> anything else.
>> This is the entire point of this exercise: allow replacing the raw
>> block device
>> accesses with the easier to use file system API. Raw block device
>> access is also
>> file API so one could argue that this is nonsense. What I mean here
>> is that by
>> abstracting zones with files, the user does not need to do the zone
>> configuration discovery with ioctl(BLKREPORTZONES), does not need to
>> do explicit
>> zone resets with ioctl(BLKRESETZONE), does not have to "start from
>> one sector
>> and write sequentially from there" management for write() calls (i.e.
>> seeks),
>> etc. This is all replaced with the file abstraction: directory entry
>> list
>> replace zone information, truncate() replace zone reset, file current
>> position
>> replaces the application zone write pointer management.
>>
>> This simplifies implementing support of applications for zoned block
>> devices,
>> but only in cases where said applications:
>> 1) Operate with large files
>> 2) have no or only minimal need for random writes
>>
>> A perfect match for this as mentioned in the commit message are LSM-
>> tree based
>> applications such as LevelDB or RocksDB. Other examples, related,
>> include
>> Bluestore distributed object store which uses RocksDB but still has a
>> bluefs
>> layer that could be replaced with zonefs.
>>
>> As an illustration of this, Ting Yao of Huazhong University of
>> Science and
>> Technology (China) and her team modified LevelDB to work with zonefs.
>> The early
>> prototype code is on github here: https://github.com/PDS-Lab/GearDB/t
>> ree/zonefs
>>
>> LSM-Tree applications typically operate on large files, in the same
>> range as
>> zoned block device zone size (e.g. 256 MB or so). While this is
>> generally a
>> parameter that can be changed, the use of zonefs and zoned block
>> device forces
>> using the zone size as the SSTable file maximum size. This can have
>> an impact on
>> the DB performance depending on the device type, but that is another
>> discussion.
>> The point here is the code simplifications that zonefs allows.
>>
>> For more general purpose use cases (small files, lots of random
>> modifications),
>> we already have the dm-zoned device mapper and f2fs support and we
>> are also
>> currently working on btrfs support. These solutions are in my opinion
>> more
>> appropriate than zonefs to address the points you raised.
>>
> 
> Sounds pretty reasonable. But I still have two worries.
> 
> First of all, even modest file system could contain about 100K files on
> a volume. So, if our zone is 256 MB then we need in 24 TB storage
> device for 100K files. Even if we consider some special use-case of
> database, for example, then it's pretty easy to imagine the creation a
> lot of files. So, are we ready to provide such huge storage devices
> (especially, for the case of SSDs)?

The small file use case you are describing is not zonefs target use case. It
does not make any sense to discuss small files in the context of zonefs. If
small file is the use case needed for an application, then a "normal" file
system should be use such as f2fs or btrfs (zoned block device support is being
worked on, see patches posted on btrfs list).

As mentioned previously, zonefs goal is to represent zones of a zoned block
device with files, thus providing a simple abstraction one file == one zone and
simplifying application implementation. And this means that the only sensible
use case for zonefs is applications using large container like files. LSM-tree
based applications being a very good match in this respect.

> Secondly, the allocation scheme is too simplified for my taste and it
> could create a significant fragmentation of a volume. Again, 256 MB is
> pretty big size. So, I assume that, mostly, it will be allocated only
> one zone at first for a created file. If file grows then it means that
> it will need to allocate the two contigous zones and to move the file's
> content. Finally, it sounds for me that it is possible to create a lot
> of holes and to achieve the volume state when it exists a lot of free
> space but files will be unable to grow and it will be impossible to add
> a new data on the volume. Have you made an estimation of the suggested
> allocation scheme?

What do you mean allocation scheme ? There is none ! one file == one zone and
all files are fully provisioned and allocated on mount. zonefs does not allow
the creation of files and there is no dynamic "block allocation". Again, please
do not consider zonefs as a normal file system. It is closer to a raw block
device interface than to a fully featured file system.

Best regards.
Damien Le Moal July 16, 2019, 11:21 a.m. UTC | #10
Dave,

On 2019/07/15 10:20, Dave Chinner wrote:
> Just a few quick things as I read through this to see how it uses
> iomap....
> 
> On Fri, Jul 12, 2019 at 12:00:17PM +0900, Damien Le Moal wrote:
>> +static int zonefs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>> +			      unsigned int flags, struct iomap *iomap)
>> +{

[...]

>> +	/* An IO cannot exceed the zone size */
>> +	if (offset >= max_isize)
>> +		return -EFBIG;
> 
> So a write() call that is for a length longer than max_isize is
> going to end up being a short write? i.e. iomap_apply() will loop
> mapping the inode until either we reach the end of the user write
> or we hit max_isize?
> 
> How is userspace supposed to tell the difference between a short
> write and a write that overruns max_isize?

offset >= max_isize is already checked when zonefs_file_write_iter() is called
and the write size truncated in that function too so that a write never goes
beyond the zone size. This check here is probably redundant and not needed. I
left it temporarily to make sure I was using iomap correctly and to check bugs.

>> +static int zonefs_map_blocks(struct iomap_writepage_ctx *wpc,
>> +			     struct inode *inode, loff_t offset)
>> +{
>> +	if (offset >= wpc->iomap.offset &&
>> +	    offset < wpc->iomap.offset + wpc->iomap.length)
>> +		return 0;
>> +
>> +	memset(&wpc->iomap, 0, sizeof(wpc->iomap));
>> +	return zonefs_iomap_begin(inode, offset, INT_MAX, 0, &wpc->iomap);
> 
> Why is the write length set to INT_MAX here? What happens when we
> get a zone that is larger than 2GB? i.e. the length parameter is a
> loff_t, not an int....

Yes indeed. Since no access can go beyond the file size,
zonefs_file_max_size(inode) is the right value here I think, even though
LLONG_MAX would work too.

>> +static int zonefs_truncate_seqfile(struct inode *inode)
>> +{
>> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
>> +	int ret;
>> +
>> +	/* Serialize against page faults */
>> +	down_write(&zi->i_mmap_sem);
>> +
>> +	ret = blkdev_reset_zones(inode->i_sb->s_bdev,
>> +				 zonefs_file_addr(inode) >> SECTOR_SHIFT,
>> +				 zonefs_file_max_size(inode) >> SECTOR_SHIFT,
>> +				 GFP_KERNEL);
> 
> Not sure GFP_KERNEL is safe here. This is called holding a
> filesystem lock here, so it's not immediately clear to me if this
> can deadlock through memory reclaim or not...

Conventional zones can be written with buffered I/Os and mmap, so recursing into
the FS on the memory allocation while holding i_mmap_sem is a possibility I
think. I changed this to GFP_NOFS.


>> +static int zonefs_inode_setattr(struct dentry *dentry, struct iattr *iattr)
>> +{
>> +	struct inode *inode = d_inode(dentry);
>> +	int ret;
>> +
>> +	ret = setattr_prepare(dentry, iattr);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if ((iattr->ia_valid & ATTR_UID &&
>> +	     !uid_eq(iattr->ia_uid, inode->i_uid)) ||
>> +	    (iattr->ia_valid & ATTR_GID &&
>> +	     !gid_eq(iattr->ia_gid, inode->i_gid))) {
>> +		ret = dquot_transfer(inode, iattr);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	if (iattr->ia_valid & ATTR_SIZE) {
>> +		/* The size of conventional zone files cannot be changed */
>> +		if (zonefs_file_is_conv(inode))
>> +			return -EPERM;
>> +
>> +		/*
>> +		 * For sequential zone files, we can only allow truncating to
>> +		 * 0 size which is equivalent to a zone reset.
>> +		 */
>> +		if (iattr->ia_size != 0)
>> +			return -EPERM;
>> +
>> +		ret = zonefs_truncate_seqfile(inode);
>> +		if (ret)
>> +			return ret;
> 
> Ok, so we are calling zonefs_truncate_seqfile() holding the i_rwsem
> as well. That does tend to imply GFP_NOFS should probably be used
> for the blkdev_reset_zones() call.

Yes. Fixed. Thanks.

>> +/*
>> + * Open a file.
>> + */
>> +static int zonefs_file_open(struct inode *inode, struct file *file)
>> +{
>> +	/*
>> +	 * Note: here we can do an explicit open of the file zone,
>> +	 * on the first open of the inode. The explicit close can be
>> +	 * done on the last release (close) call for the inode.
>> +	 */
>> +
>> +	return generic_file_open(inode, file);
>> +}
> 
> Why is a wrapper needed for this?

It is not for now. As the comment says, we may want to have a wrapper like this
in the future to do a device level zone open/close in sync with file
open/release. This is not useful for SMR HDDs but probably will for NVMe zoned
namespace devices.

>> +static int zonefs_file_fsync(struct file *file, loff_t start, loff_t end,
>> +			     int datasync)
>> +{
>> +	struct inode *inode = file_inode(file);
>> +	int ret;
>> +
>> +	/*
>> +	 * Since only direct writes are allowed in sequential files, we only
>> +	 * need a device flush for these files.
>> +	 */
>> +	if (zonefs_file_is_seq(inode))
>> +		goto flush;
>> +
>> +	ret = file_write_and_wait_range(file, start, end);
>> +	if (ret == 0)
>> +		ret = file_check_and_advance_wb_err(file);
>> +	if (ret)
>> +		return ret;
> 
>> +
>> +flush:
>> +	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> 
> The goto can be avoided in this case simply:
> 
> 	if (zonefs_file_is_conv(inode)) {
> 		/* do flush */
> 	}
> 	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);

Good point. Fixed.

>> +static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>> +{
>> +	struct inode *inode = file_inode(iocb->ki_filp);
>> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
>> +	loff_t max_pos = zonefs_file_max_size(inode);
>> +	size_t count;
>> +	ssize_t ret = 0;
>> +
>> +	/*
>> +	 * Check that the read operation does not go beyond the maximum
>> +	 * file size.
>> +	 */
>> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
>> +		return -EFBIG;
>> +
>> +	/*
>> +	 * For sequential zones, limit reads to written data.
>> +	 */
>> +	if (zonefs_file_is_seq(inode))
>> +		max_pos = i_size_read(inode);
>> +	if (iocb->ki_pos >= max_pos)
>> +		return 0;
> 
> Isn't this true for both types of zone at this point? i.e. at this
> point:
> 
> 	max_pos = i_size_read(inode);
> 	if (iocb->ki_pos >= max_pos)
> 		return 0;
> 
> because i_size is either the zonefs_file_max_size() for conventional
> zones (which we've already checked) or it's the write pointer for
> a sequential zone. i.e. it's the max position for either case.

Yes, correct. I fixed it.

> 
>> +	iov_iter_truncate(to, max_pos - iocb->ki_pos);
>> +	count = iov_iter_count(to);
>> +	if (!count)
>> +		return 0;
> 
> The iov_iter should never be zero length here, because that implies
> the position was >= max_pos and that will be caught by the above
> checks...

Absolutely. Fixed.

> 
>> +	/* Direct IO reads must be aligned to device physical sector size */
>> +	if ((iocb->ki_flags & IOCB_DIRECT) &&
>> +	    ((iocb->ki_pos | count) & sbi->s_blocksize_mask))
>> +		return -EINVAL;
>> +
>> +	if (iocb->ki_flags & IOCB_NOWAIT) {
>> +		if (!inode_trylock_shared(inode))
>> +			return -EAGAIN;
>> +	} else {
>> +		inode_lock_shared(inode);
>> +	}
> 
> IIUC, write IO completion takes the inode lock to serialise file
> size updates for sequential zones. In that case, shouldn't this lock
> be taken before we do the EOF checks above?

Yes. Fixed.

>> +/*
>> + * We got a write error: get the sequenial zone information from the device to
>> + * figure out where the zone write pointer is and verify the inode size against
>> + * it.
>> + */
>> +static int zonefs_write_failed(struct inode *inode, int error)
>> +{
>> +	struct super_block *sb = inode->i_sb;
>> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
>> +	sector_t sector = zi->i_addr >> SECTOR_SHIFT;
>> +	unsigned int noio_flag;
>> +	struct blk_zone zone;
>> +	int n = 1, ret;
>> +
>> +	zonefs_warn(sb, "Updating inode zone %llu info\n", sector);
>> +
>> +	noio_flag = memalloc_noio_save();
>> +	ret = blkdev_report_zones(sb->s_bdev, sector, &zone, &n);
>> +	memalloc_noio_restore(noio_flag);
> 
> What deadlock does the memalloc_noio_save() avoid? There should be a
> comment explaining what problem memalloc_noio_save() avoids
> everywhere it is used like this. If it isn't safe to do GFP_KERNEL
> allocations here under the i_rwsem, then why would it be safe to
> do GFP_KERNEL allocations in the truncate code under the i_rwsem?

Similarly to the truncate (zone reset) code, GFP_NOFS is safer here. So I
changed the call to memalloc_nofs_save/restore and added a comment.

>> +
>> +	if (!n)
>> +		ret = -EIO;
>> +	if (ret) {
>> +		zonefs_err(sb, "Get zone %llu report failed %d\n",
>> +			   sector, ret);
>> +		return ret;
>> +	}
>> +
>> +	zi->i_wpoffset = (zone.wp - zone.start) << SECTOR_SHIFT;
>> +	if (i_size_read(inode) != zi->i_wpoffset) {
>> +		i_size_write(inode, zi->i_wpoffset);
>> +		truncate_pagecache(inode, zi->i_wpoffset);
>> +	}
> 
> This looks .... dangerous. If the write pointer was advanced, but
> the data wasn't written properly, this causes stale data exposure on
> write failure. i.e. it's not failsafe.

It is safe because the device will return zeros or the format pattern for sector
reads that are beyond the zone write pointer. No stale data will be exposed, as
long as the device can be trusted to correctly handle reads beyond the write
pointer.

> 
> I suspect that on a sequential zone write failure and the write
> pointer does not equal the offset of the write, we should consider
> the zone corrupt. Also, this is for direct IO completion for
> sequential writes, yes? So what does the page cache truncation
> acheive given that only direct IO writes are allowed to these files?

Yes, the page cache truncation can be removed.

>> +static int zonefs_dio_seqwrite_end_io(struct kiocb *iocb, ssize_t size,
>> +				      unsigned int flags)
>> +{
>> +	struct inode *inode = file_inode(iocb->ki_filp);
>> +	int ret;
>> +
>> +	inode_lock(inode);
>> +	if (size < 0)
>> +		ret = zonefs_write_failed(inode, size);
>> +	else
>> +		ret = zonefs_update_size(inode, iocb->ki_pos + size);
>> +	inode_unlock(inode);
>> +	return ret;
> 
> Shouldn't this have a check that it's being called on a sequential
> zone inode?

Yes, a check can be added but the only caller being zonefs_file_dio_aio_write()
after checking that the inode is not a conventional zone file, I thought it was
not needed.

>> +static ssize_t zonefs_file_dio_aio_write(struct kiocb *iocb,
>> +					 struct iov_iter *from)
>> +{
>> +	struct inode *inode = file_inode(iocb->ki_filp);
>> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
>> +	size_t count;
>> +
>> +	/*
>> +	 * The size of conventional zone files is fixed to the zone size.
>> +	 * So only direct writes to sequential zones need adjusting the
>> +	 * inode size on IO completion.
>> +	 */
>> +	if (zonefs_file_is_conv(inode))
>> +		return iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
>> +
>> +	/* Enforce append only sequential writes */
>> +	count = iov_iter_count(from);
>> +	if (iocb->ki_pos != zi->i_wpoffset) {
>> +		zonefs_err(inode->i_sb,
>> +			   "Unaligned write at %llu + %zu (wp %llu)\n",
>> +			   iocb->ki_pos, count, zi->i_wpoffset);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (is_sync_kiocb(iocb)) {
>> +		/*
>> +		 * Don't use the end_io callback for synchronous iocbs,
>> +		 * as we'd deadlock on i_rwsem.  Instead perform the same
>> +		 * actions manually here.
>> +		 */
>> +		count = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
>> +		if (count < 0)
>> +			return zonefs_write_failed(inode, count);
>> +		zonefs_update_size(inode, iocb->ki_pos);
>> +		return count;
> 
> Urk. This locking is nasty, and doesn't avoid the problem.....

Do you mean the stale data exposure problem ? As said above, the device
guarantees that stale data will not be returned for reads after write pointer.
Old data (data written before the last zone reset) is not readable.

> 
>> +	}
>> +
>> +	return iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
>> +			    zonefs_dio_seqwrite_end_io);
> 
> ... because I think this can deadlock.
> 
> AFAIA, the rule is that IO completion callbacks cannot take
> a lock that is held across IO submission. The reason is that
> IO can complete so fast that the submission code runs the
> completion. i.e. iomap_dio_rw() can be the function that calls
> iomap_dio_complete() and runs the IO completion.
> 
> In which case, this will deadlock because we are already holding the
> i_rwsem and the end_io completion will try to take it again.

Arrg ! Yes ! Beginner's mistake. Will rework this.
> 
> 
> 
>> +}
>> +
>> +static ssize_t zonefs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> +{
>> +	struct inode *inode = file_inode(iocb->ki_filp);
>> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
>> +	size_t count;
>> +	ssize_t ret;
>> +
>> +	/*
>> +	 * Check that the read operation does not go beyond the file
>> +	 * zone boundary.
>> +	 */
>> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
>> +		return -EFBIG;
>> +	iov_iter_truncate(from, zonefs_file_max_size(inode) - iocb->ki_pos);
>> +	count = iov_iter_count(from);
>> +
>> +	if (!count)
>> +		return 0;
>> +
>> +	/*
>> +	 * Direct IO writes are mandatory for sequential zones so that write IO
>> +	 * order is preserved. The direct writes also must be aligned to
>> +	 * device physical sector size.
>> +	 */
>> +	if (iocb->ki_flags & IOCB_DIRECT) {
>> +		if ((iocb->ki_pos | count) & sbi->s_blocksize_mask)
>> +			return -EINVAL;
>> +	} else {
>> +		if (zonefs_file_is_seq(inode))
>> +			return -EOPNOTSUPP;
> 
> zonefs_iomap_begin() returns -EIO in this case and issues a warning.
> This seems somewhat inconsistent....

Yes. The check in zonefs_iomap_begin() is probably redundant. Will remove it.

>> +	}
>> +
>> +	if (iocb->ki_flags & IOCB_NOWAIT) {
>> +		if (!inode_trylock(inode))
>> +			return -EAGAIN;
>> +	} else {
>> +		inode_lock(inode);
>> +	}
>> +
>> +	ret = generic_write_checks(iocb, from);
>> +	if (ret <= 0)
>> +		goto out;
> 
> Shouldn't this be done before the iov_iter is truncated?

Yes. Will move it.

> 
>> +
>> +	if (iocb->ki_flags & IOCB_DIRECT)
>> +		ret = zonefs_file_dio_aio_write(iocb, from);
>> +	else
>> +		ret = iomap_file_buffered_write(iocb, from, &zonefs_iomap_ops);
>> +
>> +out:
>> +	inode_unlock(inode);
>> +
>> +	if (ret > 0 && (!(iocb->ki_flags & IOCB_DIRECT))) {
>> +		iocb->ki_pos += ret;
>> +		ret = generic_write_sync(iocb, ret);
>> +	}
> 
> Hmmm. The split of checks and doing stuff between direct IO and
> buffered IO seems a bit arbitrary. e.g. the "sequential zones can
> only do append writes" is in zonefs_file_dio_aio_write(), but we
> do a check that "sequential zones can only do direct IO" here.
> 
> And then we have the sync code that can only occur on buffered IO,
> which we don't have a wrapper function for but really should.  And I
> suspect that the locking is going to have to change here because of
> the direct IO issues, so maybe it would be best to split this up
> similar to the way XFS has two completely separate functions for the
> two paths....

Yes, very good idea. I will clean this up to clarify all cases.

>> +static struct kmem_cache *zonefs_inode_cachep;
>> +
>> +static struct inode *zonefs_alloc_inode(struct super_block *sb)
>> +{
>> +	struct zonefs_inode_info *zi;
>> +
>> +	zi = kmem_cache_alloc(zonefs_inode_cachep, GFP_KERNEL);
>> +	if (!zi)
>> +		return NULL;
>> +
>> +	init_rwsem(&zi->i_mmap_sem);
>> +	inode_init_once(&zi->i_vnode);
>> +
>> +	return &zi->i_vnode;
>> +}
>> +
>> +static void zonefs_destroy_cb(struct rcu_head *head)
>> +{
>> +	struct inode *inode = container_of(head, struct inode, i_rcu);
>> +
>> +	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
>> +}
>> +
>> +static void zonefs_destroy_inode(struct inode *inode)
>> +{
>> +	call_rcu(&inode->i_rcu, zonefs_destroy_cb);
>> +}
> 
> If this is all the inode destructor is, then implement ->free_inode
> instead. i.e.
> 
> zonefs_free_inode(inode)
> {
> 	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
> }
> 
> and the VFS takes care of the RCU freeing of the inode.

Yes, I remember now that the rcu free inode code was moved as generic in VFS a
while back. I will clean this up.

> 
>> +/*
>> + * File system stat.
>> + */
>> +static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
>> +{
>> +	struct super_block *sb = dentry->d_sb;
>> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
>> +	sector_t nr_sectors = sb->s_bdev->bd_part->nr_sects;
>> +	enum zonefs_ztype t;
>> +
>> +	buf->f_type = ZONEFS_MAGIC;
>> +	buf->f_bsize = dentry->d_sb->s_blocksize;
>> +	buf->f_namelen = ZONEFS_NAME_MAX;
>> +
>> +	buf->f_blocks = nr_sectors >> (sb->s_blocksize_bits - SECTOR_SHIFT);
>> +	buf->f_bfree = 0;
>> +	buf->f_bavail = 0;
>> +
>> +	buf->f_files = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - 1;
>> +	for (t = ZONEFS_ZTYPE_ALL; t < ZONEFS_ZTYPE_MAX; t++) {
>> +		if (sbi->s_nr_zones[t])
>> +			buf->f_files++;
>> +	}
>> +	buf->f_ffree = 0;
>> +
>> +	/* buf->f_fsid = 0; uuid, see ext2 */
> 
> This doesn't tell me anything useful. Does it mean "we should use
> the uuid like ext2" or something else? is it a "TODO:" item?

Basically yes. Forgot to code it after too much copy-paste :)
Will fix that.

>> +static char *zgroups_name[ZONEFS_ZTYPE_MAX] = {
>> +	NULL,
>> +	"cnv",
>> +	"seq"
>> +};
> 
> What's the reason for a NULL in the first entry?

This is for ZONEFS_ZTYPE_ALL, which does not have a directory. The NULL is only
there because the array is using ZONEFS_ZTYPE_MAX. This can be simplified.

> 
>> +
>> +/*
>> + * Create a zone group and populate it with zone files.
>> + */
>> +static int zonefs_create_zgroup(struct super_block *sb, struct blk_zone *zones,
>> +				enum zonefs_ztype type)
>> +{
>> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
>> +	struct blk_zone *zone, *next, *end;
>> +	char name[ZONEFS_NAME_MAX];
>> +	unsigned int nr_files = 0;
>> +	struct dentry *dir;
>> +
>> +	/* If the group is empty, nothing to do */
>> +	if (!sbi->s_nr_zones[type])
>> +		return 0;
>> +
>> +	dir = zonefs_create_inode(sb->s_root, zgroups_name[type], NULL);
>> +	if (!dir)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * Note: The first zone contains the super block: skip it.
>> +	 */
>> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
>> +	for (zone = &zones[1]; zone < end; zone = next) {
>> +
>> +		next = zone + 1;
>> +		if (zonefs_zone_type(zone) != type)
>> +			continue;
>> +
>> +		/* Ignore offline zones */
>> +		if (zonefs_zone_offline(zone))
>> +			continue;
>> +
>> +		/*
>> +		 * For conventional zones, contiguous zones can be aggregated
>> +		 * together to form larger files.
>> +		 * Note that this overwrites the length of the first zone of
>> +		 * the set of contiguous zones aggregated together.
>> +		 * Only zones with the same condition can be agreggated so that
>> +		 * offline zones are excluded and readonly zones are aggregated
>> +		 * together into a read only file.
>> +		 */
>> +		if (type == ZONEFS_ZTYPE_CNV &&
>> +		    zonefs_has_feature(sbi, ZONEFS_F_AGRCNV)) {
>> +			for (; next < end; next++) {
>> +				if (zonefs_zone_type(next) != type ||
>> +				    next->cond != zone->cond)
>> +					break;
>> +				zone->len += next->len;
>> +			}
>> +		}
>> +
>> +		if (zonefs_has_feature(sbi, ZONEFS_F_STARTSECT_NAME))
>> +			/* Use zone start sector as file names */
>> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%llu",
>> +				 zone->start);
>> +		else
>> +			/* Use file number as file names */
>> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%u", nr_files);
>> +		nr_files++;
>> +
>> +		if (!zonefs_create_inode(dir, name, zone))
>> +			return -ENOMEM;
> 
> I guess this means partial setup due to failure needs to be torn
> down by the kill_super() code?

Yes. And the call to d_genocide(sb->s_root) does that unless I missed something.

>> +static struct blk_zone *zonefs_get_zone_info(struct super_block *sb)
>> +{
>> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
>> +	struct block_device *bdev = sb->s_bdev;
>> +	sector_t nr_sectors = bdev->bd_part->nr_sects;
>> +	unsigned int i, n, nr_zones = 0;
>> +	struct blk_zone *zones, *zone;
>> +	sector_t sector = 0;
>> +	int ret;
>> +
>> +	sbi->s_blocksize_mask = sb->s_blocksize - 1;
>> +	sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] = blkdev_nr_zones(bdev);
>> +	zones = kvcalloc(sbi->s_nr_zones[ZONEFS_ZTYPE_ALL],
>> +			 sizeof(struct blk_zone), GFP_KERNEL);
>> +	if (!zones)
>> +		return ERR_PTR(-ENOMEM);
> 
> Hmmm. That's a big allocation. That might be several megabytes for a
> typical 16TB SMR drive, right? It might be worth adding a comment
> indicating just how large this is, because it's somewhat unusual in
> kernel space, even for temporary storage.

Yes. A full zone report (64B per zone) will take to 4-5MB on a 15+ TB disk. I
will comment this. Doing the big allocation to allow having all zones on hand
before creating the inodes makes the code easier. But this can be optimized with
a partial report in a loop too. I will rewrite this code to avoid the big
temporary allocation.

>> + * defined in linux/blkzoned.h, that is, BLK_ZONE_TYPE_SEQWRITE_REQ and
>> + * BLK_ZONE_TYPE_SEQWRITE_PREF.
>> + */
>> +enum zonefs_ztype {
>> +	ZONEFS_ZTYPE_ALL = 0,
>> +	ZONEFS_ZTYPE_CNV,
>> +	ZONEFS_ZTYPE_SEQ,
>> +	ZONEFS_ZTYPE_MAX,
>> +};
> 
> What is ZONEFS_ZTYPE_ALL supposed to be used for?

It is defined only to allow thee declaration of sbi->s_nr_zones as an array of
ZONEFS_ZTYPE_MAX unsigned int, so that we get 3 counters: total number of zones
is always larger or equal to conv zones + seq zones. Initially, this made the
code somewhat cleaner, but I realize now not really easier to understand :)

I will remove it and make things more obvious.

>> +static inline bool zonefs_zone_offline(struct blk_zone *zone)
>> +{
>> +	return zone->cond == BLK_ZONE_COND_OFFLINE;
>> +}
>> +
>> +static inline bool zonefs_zone_readonly(struct blk_zone *zone)
>> +{
>> +	return zone->cond == BLK_ZONE_COND_READONLY;
>> +}
> 
> These should be block layer helpers as the operate on blk_zone,
> not zonefs structures.

Yes. Will fix that.

>> +/*
>> + * Address (byte offset) on disk of a file zone.
>> + */
>> +static inline loff_t zonefs_file_addr(struct inode *inode)
>> +{
>> +	return ZONEFS_I(inode)->i_addr;
>> +}
> 
> so it's a disk address, but it's encoded in bytes rather than sectors
> so that makes it an offset. That's kinda confusing coming from a
> filesystem that makes a clear distinction between these two things.

Yes. This simplifies a little the code, but it is not obvious. I will rework this.

>> +
>> +/*
>> + * Maximum possible size of a file (i.e. the zone size).
>> + */
>> +static inline loff_t zonefs_file_max_size(struct inode *inode)
>> +{
>> +	return ZONEFS_I(inode)->i_max_size;
>> +}
>> +
>> +/*
>> + * On-disk super block (block 0).
>> + */
>> +struct zonefs_super {
>> +
>> +	/* Magic number */
>> +	__le32		s_magic;		/*    4 */
>> +
>> +	/* Metadata version number */
>> +	__le32		s_version;		/*    8 */
>> +
>> +	/* Features */
>> +	__le64		s_features;		/*   16 */
> 
> On-disk version numbers are kinda redundant when you have
> fine grained feature fields to indicate the on-disk layout...

Yes. I guess. I was kind of reserving the s_features field for optional things,
hence the version number for on-disk mandatory things. But I may as well use it
for everything, both mandatory format features and options.

>> +/*
>> + * Feature flags.
>> + */
>> +enum zonefs_features {
>> +	/*
>> +	 * Use a zone start sector value as file name.
>> +	 */
>> +	ZONEFS_F_STARTSECT_NAME,
>> +	/*
>> +	 * Aggregate contiguous conventional zones into a single file.
>> +	 */
>> +	ZONEFS_F_AGRCNV,
>> +	/*
>> +	 * Use super block specified UID for files instead of default.
>> +	 */
>> +	ZONEFS_F_UID,
>> +	/*
>> +	 * Use super block specified GID for files instead of default.
>> +	 */
>> +	ZONEFS_F_GID,
>> +	/*
>> +	 * Use super block specified file permissions instead of default 640.
>> +	 */
>> +	ZONEFS_F_PERM,
>> +};
> 
> Are these the on-disk feature bit definitions, or just used in
> memory? Or both?

These are used for both on-disk super block and in-memory sb info.

Thank you very much for the detailed review. I will rework this and repost.

Best regards.
Viacheslav Dubeyko July 16, 2019, 4:51 p.m. UTC | #11
On Mon, 2019-07-15 at 23:53 +0000, Damien Le Moal wrote:
> On 2019/07/16 1:54, Viacheslav Dubeyko wrote:
> [...]
> > 
> > > 
> > > > 
> > > > Do you have in mind some special use-case?
> > > As the commit message mentions, zonefs is not a traditional file
> > > system by any
> > > mean and much closer to a raw block device access interface than
> > > anything else.
> > > This is the entire point of this exercise: allow replacing the
> > > raw
> > > block device
> > > accesses with the easier to use file system API. Raw block device
> > > access is also
> > > file API so one could argue that this is nonsense. What I mean
> > > here
> > > is that by
> > > abstracting zones with files, the user does not need to do the
> > > zone
> > > configuration discovery with ioctl(BLKREPORTZONES), does not need
> > > to
> > > do explicit
> > > zone resets with ioctl(BLKRESETZONE), does not have to "start
> > > from
> > > one sector
> > > and write sequentially from there" management for write() calls
> > > (i.e.
> > > seeks),
> > > etc. This is all replaced with the file abstraction: directory
> > > entry
> > > list
> > > replace zone information, truncate() replace zone reset, file
> > > current
> > > position
> > > replaces the application zone write pointer management.
> > > 
> > > This simplifies implementing support of applications for zoned
> > > block
> > > devices,
> > > but only in cases where said applications:
> > > 1) Operate with large files
> > > 2) have no or only minimal need for random writes
> > > 
> > > A perfect match for this as mentioned in the commit message are
> > > LSM-
> > > tree based
> > > applications such as LevelDB or RocksDB. Other examples, related,
> > > include
> > > Bluestore distributed object store which uses RocksDB but still
> > > has a
> > > bluefs
> > > layer that could be replaced with zonefs.
> > > 
> > > As an illustration of this, Ting Yao of Huazhong University of
> > > Science and
> > > Technology (China) and her team modified LevelDB to work with
> > > zonefs.
> > > The early
> > > prototype code is on github here: https://github.com/PDS-Lab/Gear
> > > DB/t
> > > ree/zonefs
> > > 
> > > LSM-Tree applications typically operate on large files, in the
> > > same
> > > range as
> > > zoned block device zone size (e.g. 256 MB or so). While this is
> > > generally a
> > > parameter that can be changed, the use of zonefs and zoned block
> > > device forces
> > > using the zone size as the SSTable file maximum size. This can
> > > have
> > > an impact on
> > > the DB performance depending on the device type, but that is
> > > another
> > > discussion.
> > > The point here is the code simplifications that zonefs allows.
> > > 
> > > For more general purpose use cases (small files, lots of random
> > > modifications),
> > > we already have the dm-zoned device mapper and f2fs support and
> > > we
> > > are also
> > > currently working on btrfs support. These solutions are in my
> > > opinion
> > > more
> > > appropriate than zonefs to address the points you raised.
> > > 
> > Sounds pretty reasonable. But I still have two worries.
> > 
> > First of all, even modest file system could contain about 100K
> > files on
> > a volume. So, if our zone is 256 MB then we need in 24 TB storage
> > device for 100K files. Even if we consider some special use-case of
> > database, for example, then it's pretty easy to imagine the
> > creation a
> > lot of files. So, are we ready to provide such huge storage devices
> > (especially, for the case of SSDs)?
> The small file use case you are describing is not zonefs target use
> case. It
> does not make any sense to discuss small files in the context of
> zonefs. If
> small file is the use case needed for an application, then a "normal"
> file
> system should be use such as f2fs or btrfs (zoned block device
> support is being
> worked on, see patches posted on btrfs list).
> 
> As mentioned previously, zonefs goal is to represent zones of a zoned
> block
> device with files, thus providing a simple abstraction one file ==
> one zone and
> simplifying application implementation. And this means that the only
> sensible
> use case for zonefs is applications using large container like files.
> LSM-tree
> based applications being a very good match in this respect.
> 


I am talking not about file size but about number of files on the
volume here. I meant that file system could easily contain about
100,000 files on the volume. So, if every file uses 256 MB zone then
100,000 files need in 24 TB volume.


> > 
> > Secondly, the allocation scheme is too simplified for my taste and
> > it
> > could create a significant fragmentation of a volume. Again, 256 MB
> > is
> > pretty big size. So, I assume that, mostly, it will be allocated
> > only
> > one zone at first for a created file. If file grows then it means
> > that
> > it will need to allocate the two contigous zones and to move the
> > file's
> > content. Finally, it sounds for me that it is possible to create a
> > lot
> > of holes and to achieve the volume state when it exists a lot of
> > free
> > space but files will be unable to grow and it will be impossible to
> > add
> > a new data on the volume. Have you made an estimation of the
> > suggested
> > allocation scheme?
> What do you mean allocation scheme ? There is none ! one file == one
> zone and
> all files are fully provisioned and allocated on mount. zonefs does
> not allow
> the creation of files and there is no dynamic "block allocation".
> Again, please
> do not consider zonefs as a normal file system. It is closer to a raw
> block
> device interface than to a fully featured file system.
> 

OK. It sounds that a file cannot grow beyond the allocated number of
contigous zone(s) during the mount operation. Am I correct? But if a
file is needed to be resized what can be done in such case? Should it
need to re-mount the file system?

By the way, does this approach provides the way to use the device's
internal parallelism? What should anybody take into account for
exploiting the device's internal parallelism?

Thanks,
Viacheslav Dubeyko.
Damien Le Moal July 18, 2019, 12:57 a.m. UTC | #12
Slava,

On 2019/07/17 1:51, Viacheslav Dubeyko wrote:
>> As mentioned previously, zonefs goal is to represent zones of a zoned
>> block
>> device with files, thus providing a simple abstraction one file ==
>> one zone and
>> simplifying application implementation. And this means that the only
>> sensible
>> use case for zonefs is applications using large container like files.
>> LSM-tree
>> based applications being a very good match in this respect.
>>
> 
> 
> I am talking not about file size but about number of files on the
> volume here. I meant that file system could easily contain about
> 100,000 files on the volume. So, if every file uses 256 MB zone then
> 100,000 files need in 24 TB volume.

zonefs provides a different representation of the raw device. It is not
abstracting it. One file is one zone. So if the use case needs more files, then
another device model must be used (higher capacity or smaller zone size). It is
as simple as that.

>> What do you mean allocation scheme ? There is none ! one file == one
>> zone and
>> all files are fully provisioned and allocated on mount. zonefs does
>> not allow
>> the creation of files and there is no dynamic "block allocation".
>> Again, please
>> do not consider zonefs as a normal file system. It is closer to a raw
>> block
>> device interface than to a fully featured file system.
>>
> 
> OK. It sounds that a file cannot grow beyond the allocated number of
> contigous zone(s) during the mount operation. Am I correct? But if a
> file is needed to be resized what can be done in such case? Should it
> need to re-mount the file system?

In the case of sequential zone files, one file always represents a single zone.
In the case of conventional zones, the default behavior is the same, and
optionally one file can be a set of contiguous conventional zones. And a remount
can switch between one conventional zone per file or aggregated conventional
zones files. Conventional zone files have a fixed size set to the zone size.
These files cannot be truncated. For sequential zone files, only truncation to 0
is possible. That is equivalent to doing a zone reset.

A remount will not allow resizing the maximum size of files because that is
determine by the device zone size, which is fixed and cannot be changed.

> By the way, does this approach provides the way to use the device's
> internal parallelism? What should anybody take into account for
> exploiting the device's internal parallelism?

zonefs uses the standard BIO interface which does not have any provision for
exposing HW specific parallel resources. So no, there is no such feature
implemented. Zoned block devices are for now SMR HDDs only anyway, and HDDs are
have no parallelism.
Jeff Moyer July 18, 2019, 2:11 p.m. UTC | #13
Hi, Damien,

Did you consider creating a shared library?  I bet that would also
ease application adoption for the use cases you're interested in, and
would have similar performance.

-Jeff

Damien Le Moal <damien.lemoal@wdc.com> writes:

> zonefs is a very simple file system exposing each zone of a zoned
> block device as a file. This is intended to simplify implementation of
> application zoned block device raw access support by allowing switching
> to the well known POSIX file API rather than relying on direct block
> device file ioctls and read/write. Zonefs, for instance, greatly
> simplifies the implementation of LSM (log-structured merge) tree
> structures (such as used in RocksDB and LevelDB) on zoned block devices
> by allowing SSTables to be stored in a zone file similarly to a regular
> file system architecture, hence reducing the amount of change needed in
> the application.
>
> Due to its simplicity, zonefs is in fact closer to a raw block device
> access interface than to a full feature POSIX file system. This RFC
> implementation uses Christoph's work on generic iomap writepage
> implementation and is based on Christoph's gfs2 tree available at
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/gfs2-iomap
>
> Zonefs on-disk metadata is reduced to a super block to store a magic
> number, a uuid and optional features flags and values. On mount, zonefs
> uses blkdev_report_zones() to obtain the device zone configuration and
> populates the mount point with a static file tree solely based on this
> information. E.g. file sizes come from zone write pointer offset managed
> by the device itself.
>
> The zone files created on mount have the following characteristics.
> 1) Files representing zones of the same type are grouped together
>    under a common directory:
>   * For conventional zones, the directory "cnv" is used.
>   * For sequential write zones, the directory "seq" is used.
>   These two directories are the only directories that exist in zonefs.
>   Users cannot create other directories and cannot rename nor delete
>   the "cnv" and "seq" directories.
> 2) The name of zone files is by default the number of the file within
>    the zone type directory, in order of increasing zone start sector.
> 3) The size of conventional zone files is fixed to the device zone size.
>    Conventional zone files cannot be truncated.
> 4) The size of sequential zone files represent the file zone write
>    pointer position relative to the zone start sector. Truncating these
>    files is allowed only down to 0, in wich case, the zone is reset to
>    rewind the file zone write pointer position to the start of the zone.
> 5) All read and write operations to files are not allowed beyond the
>    file zone size. Any access exceeding the zone size is failed with
>    the -EFBIG error.
> 6) Creating, deleting, renaming or modifying any attribute of files
>    and directories is not allowed. The only exception being the file
>    size of sequential zone files which can be modified by write
>    operations or truncation to 0.
>
> Several optional features of zonefs can be enabled at format time.
> * Conventional zone aggregation: contiguous conventional zones can be
>   agregated into a single larger file instead of multiple per-zone
>   files.
> * File naming: the default file number file name can be switched to
>   using the base-10 value of the file zone start sector.
> * File ownership: The owner UID and GID of zone files is by default 0
>   (root) but can be changed to any valid UID/GID.
> * File access permissions: the default 640 access permissions can be
>   changed.
>
> The mkzonefs tool is used to format zonefs. This tool will be available
> on Github at:
>
> https://github.com/westerndigitalcorporation/zonefs-tools
>
> Example: the following formats a host-managed SMR HDD with the
> conventional zone aggregation feature enabled.
>
> mkzonefs -o aggr_cnv /dev/sdX
> mount -t zonefs /dev/sdX /mnt
> ls -l /mnt/
> total 0
> dr-xr-xr-x 2 root root 0 Apr 11 13:00 cnv
> dr-xr-xr-x 2 root root 0 Apr 11 13:00 seq
>
> ls -l /mnt/cnv
> total 137363456
> -rw-rw---- 1 root root 140660178944 Apr 11 13:00 0
>
> ls -Fal -v /mnt/seq
> total 14511243264
> dr-xr-xr-x 2 root root 15942528 Jul 10 11:53 ./
> drwxr-xr-x 4 root root     1152 Jul 10 11:53 ../
> -rw-r----- 1 root root        0 Jul 10 11:53 0
> -rw-r----- 1 root root 33554432 Jul 10 13:43 1
> -rw-r----- 1 root root        0 Jul 10 11:53 2
> -rw-r----- 1 root root        0 Jul 10 11:53 3
> ...
>
> The aggregated conventional zone file can be used as a regular file.
> Operations such as the following work.
>
> mkfs.ext4 /mnt/cnv/0
> mount -o loop /mnt/cnv/0 /data
>
> Contains contributions from Johannes Thumshirn <jthumshirn@suse.de>
> and Christoph Hellwig <hch@lst.de>.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  fs/Kconfig                 |    2 +
>  fs/Makefile                |    1 +
>  fs/zonefs/Kconfig          |    9 +
>  fs/zonefs/Makefile         |    4 +
>  fs/zonefs/super.c          | 1004 ++++++++++++++++++++++++++++++++++++
>  fs/zonefs/zonefs.h         |  190 +++++++
>  include/uapi/linux/magic.h |    1 +
>  7 files changed, 1211 insertions(+)
>  create mode 100644 fs/zonefs/Kconfig
>  create mode 100644 fs/zonefs/Makefile
>  create mode 100644 fs/zonefs/super.c
>  create mode 100644 fs/zonefs/zonefs.h
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index f1046cf6ad85..e48cc0e0efbb 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -41,6 +41,7 @@ source "fs/ocfs2/Kconfig"
>  source "fs/btrfs/Kconfig"
>  source "fs/nilfs2/Kconfig"
>  source "fs/f2fs/Kconfig"
> +source "fs/zonefs/Kconfig"
>  
>  config FS_DAX
>  	bool "Direct Access (DAX) support"
> @@ -262,6 +263,7 @@ source "fs/romfs/Kconfig"
>  source "fs/pstore/Kconfig"
>  source "fs/sysv/Kconfig"
>  source "fs/ufs/Kconfig"
> +source "fs/ufs/Kconfig"
>  
>  endif # MISC_FILESYSTEMS
>  
> diff --git a/fs/Makefile b/fs/Makefile
> index c9aea23aba56..02fcd528991b 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -130,3 +130,4 @@ obj-$(CONFIG_F2FS_FS)		+= f2fs/
>  obj-$(CONFIG_CEPH_FS)		+= ceph/
>  obj-$(CONFIG_PSTORE)		+= pstore/
>  obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
> +obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> diff --git a/fs/zonefs/Kconfig b/fs/zonefs/Kconfig
> new file mode 100644
> index 000000000000..6490547e9763
> --- /dev/null
> +++ b/fs/zonefs/Kconfig
> @@ -0,0 +1,9 @@
> +config ZONEFS_FS
> +	tristate "zonefs filesystem support"
> +	depends on BLOCK
> +	depends on BLK_DEV_ZONED
> +	help
> +	  zonefs is a simple File System which exposes zones of a zoned block
> +	  device as files.
> +
> +	  If unsure, say N.
> diff --git a/fs/zonefs/Makefile b/fs/zonefs/Makefile
> new file mode 100644
> index 000000000000..75a380aa1ae1
> --- /dev/null
> +++ b/fs/zonefs/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_ZONEFS_FS) += zonefs.o
> +
> +zonefs-y	:= super.o
> diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
> new file mode 100644
> index 000000000000..0bf56f378a77
> --- /dev/null
> +++ b/fs/zonefs/super.c
> @@ -0,0 +1,1004 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple zone file system for zoned block devices.
> + *
> + * Copyright (C) 2019 Western Digital Corporation or its affiliates.
> + */
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/magic.h>
> +#include <linux/iomap.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/blkdev.h>
> +#include <linux/statfs.h>
> +#include <linux/writeback.h>
> +#include <linux/quotaops.h>
> +#include <linux/seq_file.h>
> +#include <linux/parser.h>
> +#include <linux/uio.h>
> +#include <linux/mman.h>
> +#include <linux/sched/mm.h>
> +
> +#include "zonefs.h"
> +
> +static int zonefs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> +			      unsigned int flags, struct iomap *iomap)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	loff_t max_isize = zonefs_file_max_size(inode);
> +	loff_t isize = i_size_read(inode);
> +
> +	/*
> +	 * For sequential zones, enforce direct IO writes. This is already
> +	 * checked when writes are issued, so warn about this here if we
> +	 * get buffered write to a sequential file inode.
> +	 */
> +	if (WARN_ON_ONCE(zonefs_file_is_seq(inode) && (flags & IOMAP_WRITE) &&
> +			 (!(flags & IOMAP_DIRECT))))
> +		return -EIO;
> +
> +	/* An IO cannot exceed the zone size */
> +	if (offset >= max_isize)
> +		return -EFBIG;
> +
> +	/* All blocks are always mapped */
> +	if (offset >= i_size_read(inode)) {
> +		length = min(length, max_isize - offset);
> +		iomap->type = IOMAP_UNWRITTEN;
> +	} else {
> +		length = min(length, isize - offset);
> +		iomap->type = IOMAP_MAPPED;
> +	}
> +	iomap->offset = offset & (~sbi->s_blocksize_mask);
> +	iomap->length = (offset + length + sbi->s_blocksize_mask) &
> +			(~sbi->s_blocksize_mask);
> +	iomap->addr = zonefs_file_addr(inode) + iomap->offset;
> +	iomap->bdev = inode->i_sb->s_bdev;
> +
> +	return 0;
> +}
> +
> +static const struct iomap_ops zonefs_iomap_ops = {
> +	.iomap_begin	= zonefs_iomap_begin,
> +};
> +
> +static int zonefs_readpage(struct file *unused, struct page *page)
> +{
> +	return iomap_readpage(page, &zonefs_iomap_ops);
> +}
> +
> +static int zonefs_readpages(struct file *unused, struct address_space *mapping,
> +			    struct list_head *pages, unsigned int nr_pages)
> +{
> +	return iomap_readpages(mapping, pages, nr_pages, &zonefs_iomap_ops);
> +}
> +
> +static int zonefs_map_blocks(struct iomap_writepage_ctx *wpc,
> +			     struct inode *inode, loff_t offset)
> +{
> +	if (offset >= wpc->iomap.offset &&
> +	    offset < wpc->iomap.offset + wpc->iomap.length)
> +		return 0;
> +
> +	memset(&wpc->iomap, 0, sizeof(wpc->iomap));
> +	return zonefs_iomap_begin(inode, offset, INT_MAX, 0, &wpc->iomap);
> +}
> +
> +static const struct iomap_writeback_ops zonefs_writeback_ops = {
> +	.map_blocks		= zonefs_map_blocks,
> +};
> +
> +static int zonefs_writepage(struct page *page, struct writeback_control *wbc)
> +{
> +	struct iomap_writepage_ctx wpc = { };
> +
> +	return iomap_writepage(page, wbc, &wpc, &zonefs_writeback_ops);
> +}
> +
> +static int zonefs_writepages(struct address_space *mapping,
> +			     struct writeback_control *wbc)
> +{
> +	struct iomap_writepage_ctx wpc = { };
> +
> +	return iomap_writepages(mapping, wbc, &wpc, &zonefs_writeback_ops);
> +}
> +
> +static const struct address_space_operations zonefs_file_aops = {
> +	.readpage		= zonefs_readpage,
> +	.readpages		= zonefs_readpages,
> +	.writepage		= zonefs_writepage,
> +	.writepages		= zonefs_writepages,
> +	.set_page_dirty		= iomap_set_page_dirty,
> +	.releasepage		= iomap_releasepage,
> +	.invalidatepage		= iomap_invalidatepage,
> +	.migratepage		= iomap_migrate_page,
> +	.is_partially_uptodate  = iomap_is_partially_uptodate,
> +	.error_remove_page	= generic_error_remove_page,
> +	.direct_IO		= noop_direct_IO,
> +};
> +
> +static int zonefs_truncate_seqfile(struct inode *inode)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	int ret;
> +
> +	/* Serialize against page faults */
> +	down_write(&zi->i_mmap_sem);
> +
> +	ret = blkdev_reset_zones(inode->i_sb->s_bdev,
> +				 zonefs_file_addr(inode) >> SECTOR_SHIFT,
> +				 zonefs_file_max_size(inode) >> SECTOR_SHIFT,
> +				 GFP_KERNEL);
> +	if (ret) {
> +		zonefs_err(inode->i_sb,
> +			   "zonefs: Reset zone at %llu failed %d",
> +			   zonefs_file_addr(inode) >> SECTOR_SHIFT,
> +			   ret);
> +	} else {
> +		truncate_setsize(inode, 0);
> +		zi->i_wpoffset = 0;
> +	}
> +
> +	up_write(&zi->i_mmap_sem);
> +
> +	return ret;
> +}
> +
> +static int zonefs_inode_setattr(struct dentry *dentry, struct iattr *iattr)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	int ret;
> +
> +	ret = setattr_prepare(dentry, iattr);
> +	if (ret)
> +		return ret;
> +
> +	if ((iattr->ia_valid & ATTR_UID &&
> +	     !uid_eq(iattr->ia_uid, inode->i_uid)) ||
> +	    (iattr->ia_valid & ATTR_GID &&
> +	     !gid_eq(iattr->ia_gid, inode->i_gid))) {
> +		ret = dquot_transfer(inode, iattr);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (iattr->ia_valid & ATTR_SIZE) {
> +		/* The size of conventional zone files cannot be changed */
> +		if (zonefs_file_is_conv(inode))
> +			return -EPERM;
> +
> +		/*
> +		 * For sequential zone files, we can only allow truncating to
> +		 * 0 size which is equivalent to a zone reset.
> +		 */
> +		if (iattr->ia_size != 0)
> +			return -EPERM;
> +
> +		ret = zonefs_truncate_seqfile(inode);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	setattr_copy(inode, iattr);
> +
> +	return 0;
> +}
> +
> +static const struct inode_operations zonefs_file_inode_operations = {
> +	.setattr	= zonefs_inode_setattr,
> +};
> +
> +/*
> + * Open a file.
> + */
> +static int zonefs_file_open(struct inode *inode, struct file *file)
> +{
> +	/*
> +	 * Note: here we can do an explicit open of the file zone,
> +	 * on the first open of the inode. The explicit close can be
> +	 * done on the last release (close) call for the inode.
> +	 */
> +
> +	return generic_file_open(inode, file);
> +}
> +
> +static int zonefs_file_fsync(struct file *file, loff_t start, loff_t end,
> +			     int datasync)
> +{
> +	struct inode *inode = file_inode(file);
> +	int ret;
> +
> +	/*
> +	 * Since only direct writes are allowed in sequential files, we only
> +	 * need a device flush for these files.
> +	 */
> +	if (zonefs_file_is_seq(inode))
> +		goto flush;
> +
> +	ret = file_write_and_wait_range(file, start, end);
> +	if (ret == 0)
> +		ret = file_check_and_advance_wb_err(file);
> +	if (ret)
> +		return ret;
> +
> +flush:
> +	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> +}
> +
> +static vm_fault_t zonefs_filemap_fault(struct vm_fault *vmf)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(file_inode(vmf->vma->vm_file));
> +	vm_fault_t ret;
> +
> +	down_read(&zi->i_mmap_sem);
> +	ret = filemap_fault(vmf);
> +	up_read(&zi->i_mmap_sem);
> +
> +	return ret;
> +}
> +
> +static vm_fault_t zonefs_filemap_page_mkwrite(struct vm_fault *vmf)
> +{
> +	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	vm_fault_t ret;
> +
> +	sb_start_pagefault(inode->i_sb);
> +	file_update_time(vmf->vma->vm_file);
> +
> +	/* Serialize against truncates */
> +	down_read(&zi->i_mmap_sem);
> +	ret = iomap_page_mkwrite(vmf, &zonefs_iomap_ops);
> +	up_read(&zi->i_mmap_sem);
> +
> +	sb_end_pagefault(inode->i_sb);
> +	return ret;
> +}
> +
> +static const struct vm_operations_struct zonefs_file_vm_ops = {
> +	.fault		= zonefs_filemap_fault,
> +	.map_pages	= filemap_map_pages,
> +	.page_mkwrite	= zonefs_filemap_page_mkwrite,
> +};
> +
> +static int zonefs_file_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	/*
> +	 * Conventional zone files can be mmap-ed READ/WRITE.
> +	 * For sequential zone files, only readonly mappings are possible.
> +	 */
> +	if (zonefs_file_is_seq(file_inode(file)) &&
> +	    (vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
> +		return -EINVAL;
> +
> +	file_accessed(file);
> +	vma->vm_ops = &zonefs_file_vm_ops;
> +
> +	return 0;
> +}
> +
> +static loff_t zonefs_file_llseek(struct file *file, loff_t offset, int whence)
> +{
> +	loff_t isize = i_size_read(file_inode(file));
> +
> +	/*
> +	 * Seeks are limited to below the zone size for conventional zones
> +	 * and below the zone write pointer for sequential zones. In both
> +	 * cases, this limit is the inode size.
> +	 */
> +	return generic_file_llseek_size(file, offset, whence, isize, isize);
> +}
> +
> +static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	loff_t max_pos = zonefs_file_max_size(inode);
> +	size_t count;
> +	ssize_t ret = 0;
> +
> +	/*
> +	 * Check that the read operation does not go beyond the maximum
> +	 * file size.
> +	 */
> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
> +		return -EFBIG;
> +
> +	/*
> +	 * For sequential zones, limit reads to written data.
> +	 */
> +	if (zonefs_file_is_seq(inode))
> +		max_pos = i_size_read(inode);
> +	if (iocb->ki_pos >= max_pos)
> +		return 0;
> +
> +	iov_iter_truncate(to, max_pos - iocb->ki_pos);
> +	count = iov_iter_count(to);
> +	if (!count)
> +		return 0;
> +
> +	/* Direct IO reads must be aligned to device physical sector size */
> +	if ((iocb->ki_flags & IOCB_DIRECT) &&
> +	    ((iocb->ki_pos | count) & sbi->s_blocksize_mask))
> +		return -EINVAL;
> +
> +	if (iocb->ki_flags & IOCB_NOWAIT) {
> +		if (!inode_trylock_shared(inode))
> +			return -EAGAIN;
> +	} else {
> +		inode_lock_shared(inode);
> +	}
> +
> +	if (iocb->ki_flags & IOCB_DIRECT) {
> +		file_accessed(iocb->ki_filp);
> +		ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, NULL);
> +	} else {
> +		ret = generic_file_read_iter(iocb, to);
> +	}
> +
> +	inode_unlock_shared(inode);
> +
> +	return ret;
> +}
> +
> +/*
> + * We got a write error: get the sequenial zone information from the device to
> + * figure out where the zone write pointer is and verify the inode size against
> + * it.
> + */
> +static int zonefs_write_failed(struct inode *inode, int error)
> +{
> +	struct super_block *sb = inode->i_sb;
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	sector_t sector = zi->i_addr >> SECTOR_SHIFT;
> +	unsigned int noio_flag;
> +	struct blk_zone zone;
> +	int n = 1, ret;
> +
> +	zonefs_warn(sb, "Updating inode zone %llu info\n", sector);
> +
> +	noio_flag = memalloc_noio_save();
> +	ret = blkdev_report_zones(sb->s_bdev, sector, &zone, &n);
> +	memalloc_noio_restore(noio_flag);
> +
> +	if (!n)
> +		ret = -EIO;
> +	if (ret) {
> +		zonefs_err(sb, "Get zone %llu report failed %d\n",
> +			   sector, ret);
> +		return ret;
> +	}
> +
> +	zi->i_wpoffset = (zone.wp - zone.start) << SECTOR_SHIFT;
> +	if (i_size_read(inode) != zi->i_wpoffset) {
> +		i_size_write(inode, zi->i_wpoffset);
> +		truncate_pagecache(inode, zi->i_wpoffset);
> +	}
> +
> +	return error;
> +}
> +
> +static int zonefs_update_size(struct inode *inode, loff_t new_pos)
> +{
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +
> +	zi->i_wpoffset = new_pos;
> +	if (new_pos > i_size_read(inode))
> +		i_size_write(inode, new_pos);
> +	return 0;
> +}
> +
> +static int zonefs_dio_seqwrite_end_io(struct kiocb *iocb, ssize_t size,
> +				      unsigned int flags)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	int ret;
> +
> +	inode_lock(inode);
> +	if (size < 0)
> +		ret = zonefs_write_failed(inode, size);
> +	else
> +		ret = zonefs_update_size(inode, iocb->ki_pos + size);
> +	inode_unlock(inode);
> +	return ret;
> +}
> +
> +static ssize_t zonefs_file_dio_aio_write(struct kiocb *iocb,
> +					 struct iov_iter *from)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +	size_t count;
> +
> +	/*
> +	 * The size of conventional zone files is fixed to the zone size.
> +	 * So only direct writes to sequential zones need adjusting the
> +	 * inode size on IO completion.
> +	 */
> +	if (zonefs_file_is_conv(inode))
> +		return iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
> +
> +	/* Enforce append only sequential writes */
> +	count = iov_iter_count(from);
> +	if (iocb->ki_pos != zi->i_wpoffset) {
> +		zonefs_err(inode->i_sb,
> +			   "Unaligned write at %llu + %zu (wp %llu)\n",
> +			   iocb->ki_pos, count, zi->i_wpoffset);
> +		return -EINVAL;
> +	}
> +
> +	if (is_sync_kiocb(iocb)) {
> +		/*
> +		 * Don't use the end_io callback for synchronous iocbs,
> +		 * as we'd deadlock on i_rwsem.  Instead perform the same
> +		 * actions manually here.
> +		 */
> +		count = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
> +		if (count < 0)
> +			return zonefs_write_failed(inode, count);
> +		zonefs_update_size(inode, iocb->ki_pos);
> +		return count;
> +	}
> +
> +	return iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
> +			    zonefs_dio_seqwrite_end_io);
> +}
> +
> +static ssize_t zonefs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
> +	size_t count;
> +	ssize_t ret;
> +
> +	/*
> +	 * Check that the read operation does not go beyond the file
> +	 * zone boundary.
> +	 */
> +	if (iocb->ki_pos >= zonefs_file_max_size(inode))
> +		return -EFBIG;
> +	iov_iter_truncate(from, zonefs_file_max_size(inode) - iocb->ki_pos);
> +	count = iov_iter_count(from);
> +
> +	if (!count)
> +		return 0;
> +
> +	/*
> +	 * Direct IO writes are mandatory for sequential zones so that write IO
> +	 * order is preserved. The direct writes also must be aligned to
> +	 * device physical sector size.
> +	 */
> +	if (iocb->ki_flags & IOCB_DIRECT) {
> +		if ((iocb->ki_pos | count) & sbi->s_blocksize_mask)
> +			return -EINVAL;
> +	} else {
> +		if (zonefs_file_is_seq(inode))
> +			return -EOPNOTSUPP;
> +	}
> +
> +	if (iocb->ki_flags & IOCB_NOWAIT) {
> +		if (!inode_trylock(inode))
> +			return -EAGAIN;
> +	} else {
> +		inode_lock(inode);
> +	}
> +
> +	ret = generic_write_checks(iocb, from);
> +	if (ret <= 0)
> +		goto out;
> +
> +	if (iocb->ki_flags & IOCB_DIRECT)
> +		ret = zonefs_file_dio_aio_write(iocb, from);
> +	else
> +		ret = iomap_file_buffered_write(iocb, from, &zonefs_iomap_ops);
> +
> +out:
> +	inode_unlock(inode);
> +
> +	if (ret > 0 && (!(iocb->ki_flags & IOCB_DIRECT))) {
> +		iocb->ki_pos += ret;
> +		ret = generic_write_sync(iocb, ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct file_operations zonefs_file_operations = {
> +	.open		= zonefs_file_open,
> +	.fsync		= zonefs_file_fsync,
> +	.mmap		= zonefs_file_mmap,
> +	.llseek		= zonefs_file_llseek,
> +	.read_iter	= zonefs_file_read_iter,
> +	.write_iter	= zonefs_file_write_iter,
> +	.splice_read	= generic_file_splice_read,
> +	.splice_write	= iter_file_splice_write,
> +	.iopoll		= iomap_dio_iopoll,
> +};
> +
> +
> +static struct kmem_cache *zonefs_inode_cachep;
> +
> +static struct inode *zonefs_alloc_inode(struct super_block *sb)
> +{
> +	struct zonefs_inode_info *zi;
> +
> +	zi = kmem_cache_alloc(zonefs_inode_cachep, GFP_KERNEL);
> +	if (!zi)
> +		return NULL;
> +
> +	init_rwsem(&zi->i_mmap_sem);
> +	inode_init_once(&zi->i_vnode);
> +
> +	return &zi->i_vnode;
> +}
> +
> +static void zonefs_destroy_cb(struct rcu_head *head)
> +{
> +	struct inode *inode = container_of(head, struct inode, i_rcu);
> +
> +	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
> +}
> +
> +static void zonefs_destroy_inode(struct inode *inode)
> +{
> +	call_rcu(&inode->i_rcu, zonefs_destroy_cb);
> +}
> +
> +static struct dentry *zonefs_create_inode(struct dentry *parent,
> +					  const char *name,
> +					  struct blk_zone *zone)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(parent->d_sb);
> +	struct inode *dir = d_inode(parent);
> +	struct dentry *dentry;
> +	struct inode *inode;
> +
> +	dentry = d_alloc_name(parent, name);
> +	if (!dentry)
> +		return NULL;
> +
> +	inode = new_inode(parent->d_sb);
> +	if (!inode)
> +		goto out_dput;
> +
> +	inode->i_ino = get_next_ino();
> +	if (zone) {
> +		struct zonefs_inode_info *zi = ZONEFS_I(inode);
> +
> +		/* Zone file */
> +		inode->i_mode = S_IFREG | sbi->s_perm;
> +		if (zonefs_zone_readonly(zone))
> +			inode->i_mode &= ~S_IWUGO;
> +		inode->i_uid = sbi->s_uid;
> +		inode->i_gid = sbi->s_gid;
> +		zi->i_ztype = zonefs_zone_type(zone);
> +		zi->i_addr = zone->start << SECTOR_SHIFT;
> +		zi->i_max_size = zone->len << SECTOR_SHIFT;
> +		inode->i_blocks = zone->len;
> +		if (zonefs_file_is_conv(inode))
> +			zi->i_wpoffset = zi->i_max_size;
> +		else
> +			zi->i_wpoffset =
> +				(zone->wp - zone->start) << SECTOR_SHIFT;
> +		inode->i_size = zi->i_wpoffset;
> +		inode->i_fop = &zonefs_file_operations;
> +		inode->i_op = &zonefs_file_inode_operations;
> +		inode->i_mapping->a_ops = &zonefs_file_aops;
> +		inode->i_mapping->a_ops = &zonefs_file_aops;
> +	} else {
> +		/* Zone group directory */
> +		inode_init_owner(inode, dir, S_IFDIR | 0555);
> +		inode->i_fop = &simple_dir_operations;
> +		inode->i_op = &simple_dir_inode_operations;
> +		set_nlink(inode, 2);
> +		inc_nlink(dir);
> +	}
> +	inode->i_ctime = inode->i_mtime = inode->i_atime = dir->i_ctime;
> +
> +	d_add(dentry, inode);
> +	d_inode(parent)->i_size += sizeof(struct dentry);
> +
> +	return dentry;
> +
> +out_dput:
> +	dput(dentry);
> +	return NULL;
> +}
> +
> +/*
> + * File system stat.
> + */
> +static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
> +{
> +	struct super_block *sb = dentry->d_sb;
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	sector_t nr_sectors = sb->s_bdev->bd_part->nr_sects;
> +	enum zonefs_ztype t;
> +
> +	buf->f_type = ZONEFS_MAGIC;
> +	buf->f_bsize = dentry->d_sb->s_blocksize;
> +	buf->f_namelen = ZONEFS_NAME_MAX;
> +
> +	buf->f_blocks = nr_sectors >> (sb->s_blocksize_bits - SECTOR_SHIFT);
> +	buf->f_bfree = 0;
> +	buf->f_bavail = 0;
> +
> +	buf->f_files = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - 1;
> +	for (t = ZONEFS_ZTYPE_ALL; t < ZONEFS_ZTYPE_MAX; t++) {
> +		if (sbi->s_nr_zones[t])
> +			buf->f_files++;
> +	}
> +	buf->f_ffree = 0;
> +
> +	/* buf->f_fsid = 0; uuid, see ext2 */
> +	buf->f_namelen = ZONEFS_NAME_MAX;
> +
> +	return 0;
> +}
> +
> +static const struct super_operations zonefs_sops = {
> +	.alloc_inode	= zonefs_alloc_inode,
> +	.destroy_inode	= zonefs_destroy_inode,
> +	.statfs		= zonefs_statfs,
> +};
> +
> +static char *zgroups_name[ZONEFS_ZTYPE_MAX] = {
> +	NULL,
> +	"cnv",
> +	"seq"
> +};
> +
> +/*
> + * Create a zone group and populate it with zone files.
> + */
> +static int zonefs_create_zgroup(struct super_block *sb, struct blk_zone *zones,
> +				enum zonefs_ztype type)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	struct blk_zone *zone, *next, *end;
> +	char name[ZONEFS_NAME_MAX];
> +	unsigned int nr_files = 0;
> +	struct dentry *dir;
> +
> +	/* If the group is empty, nothing to do */
> +	if (!sbi->s_nr_zones[type])
> +		return 0;
> +
> +	dir = zonefs_create_inode(sb->s_root, zgroups_name[type], NULL);
> +	if (!dir)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Note: The first zone contains the super block: skip it.
> +	 */
> +	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
> +	for (zone = &zones[1]; zone < end; zone = next) {
> +
> +		next = zone + 1;
> +		if (zonefs_zone_type(zone) != type)
> +			continue;
> +
> +		/* Ignore offline zones */
> +		if (zonefs_zone_offline(zone))
> +			continue;
> +
> +		/*
> +		 * For conventional zones, contiguous zones can be aggregated
> +		 * together to form larger files.
> +		 * Note that this overwrites the length of the first zone of
> +		 * the set of contiguous zones aggregated together.
> +		 * Only zones with the same condition can be agreggated so that
> +		 * offline zones are excluded and readonly zones are aggregated
> +		 * together into a read only file.
> +		 */
> +		if (type == ZONEFS_ZTYPE_CNV &&
> +		    zonefs_has_feature(sbi, ZONEFS_F_AGRCNV)) {
> +			for (; next < end; next++) {
> +				if (zonefs_zone_type(next) != type ||
> +				    next->cond != zone->cond)
> +					break;
> +				zone->len += next->len;
> +			}
> +		}
> +
> +		if (zonefs_has_feature(sbi, ZONEFS_F_STARTSECT_NAME))
> +			/* Use zone start sector as file names */
> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%llu",
> +				 zone->start);
> +		else
> +			/* Use file number as file names */
> +			snprintf(name, ZONEFS_NAME_MAX - 1, "%u", nr_files);
> +		nr_files++;
> +
> +		if (!zonefs_create_inode(dir, name, zone))
> +			return -ENOMEM;
> +	}
> +
> +	zonefs_info(sb, "Zone group %d (%s), %u zones -> %u file%s\n",
> +		    type, zgroups_name[type], sbi->s_nr_zones[type],
> +		    nr_files, nr_files > 1 ? "s" : "");
> +
> +	return 0;
> +}
> +
> +static struct blk_zone *zonefs_get_zone_info(struct super_block *sb)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	struct block_device *bdev = sb->s_bdev;
> +	sector_t nr_sectors = bdev->bd_part->nr_sects;
> +	unsigned int i, n, nr_zones = 0;
> +	struct blk_zone *zones, *zone;
> +	sector_t sector = 0;
> +	int ret;
> +
> +	sbi->s_blocksize_mask = sb->s_blocksize - 1;
> +	sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] = blkdev_nr_zones(bdev);
> +	zones = kvcalloc(sbi->s_nr_zones[ZONEFS_ZTYPE_ALL],
> +			 sizeof(struct blk_zone), GFP_KERNEL);
> +	if (!zones)
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Get zones information */
> +	zone = zones;
> +	while (nr_zones < sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] &&
> +	       sector < nr_sectors) {
> +
> +		n = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - nr_zones;
> +		ret = blkdev_report_zones(bdev, sector, zone, &n);
> +		if (ret) {
> +			zonefs_err(sb, "Zone report failed %d\n", ret);
> +			goto err;
> +		}
> +		if (!n) {
> +			zonefs_err(sb, "No zones reported\n");
> +			ret = -EIO;
> +			goto err;
> +		}
> +
> +		for (i = 0; i < n; i++) {
> +			switch (zone->type) {
> +			case BLK_ZONE_TYPE_CONVENTIONAL:
> +				zone->wp = zone->start + zone->len;
> +				if (zone > zones)
> +					sbi->s_nr_zones[ZONEFS_ZTYPE_CNV]++;
> +				break;
> +			case BLK_ZONE_TYPE_SEQWRITE_REQ:
> +			case BLK_ZONE_TYPE_SEQWRITE_PREF:
> +				if (zone > zones)
> +					sbi->s_nr_zones[ZONEFS_ZTYPE_SEQ]++;
> +				break;
> +			default:
> +				zonefs_err(sb, "Unsupported zone type 0x%x\n",
> +					   zone->type);
> +				ret = -EIO;
> +				goto err;
> +			}
> +			sector += zone->len;
> +			zone++;
> +		}
> +
> +		nr_zones += n;
> +	}
> +
> +	if (sector < nr_sectors ||
> +	    nr_zones < sbi->s_nr_zones[ZONEFS_ZTYPE_ALL]) {
> +		zonefs_err(sb, "Incomplete zone report\n");
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	return zones;
> +
> +err:
> +	kvfree(zones);
> +	return ERR_PTR(ret);
> +}
> +
> +/*
> + * Read super block information from the device.
> + */
> +static int zonefs_read_super(struct super_block *sb)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +	struct zonefs_super *super;
> +	struct bio bio;
> +	struct bio_vec bio_vec;
> +	struct page *page;
> +	int ret;
> +
> +	page = alloc_page(GFP_KERNEL);
> +	if (!page)
> +		return -ENOMEM;
> +
> +	bio_init(&bio, &bio_vec, 1);
> +	bio.bi_iter.bi_sector = 0;
> +	bio_set_dev(&bio, sb->s_bdev);
> +	bio_set_op_attrs(&bio, REQ_OP_READ, 0);
> +	bio_add_page(&bio, page, PAGE_SIZE, 0);
> +
> +	ret = submit_bio_wait(&bio);
> +	if (ret)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	super = page_address(page);
> +	if (le32_to_cpu(super->s_magic) != ZONEFS_MAGIC)
> +		goto out;
> +	if (le32_to_cpu(super->s_version) != ZONEFS_VERSION)
> +		goto out;
> +	sbi->s_features = le64_to_cpu(super->s_features);
> +	if (zonefs_has_feature(sbi, ZONEFS_F_UID)) {
> +		sbi->s_uid = make_kuid(current_user_ns(),
> +				       le32_to_cpu(super->s_uid));
> +		if (!uid_valid(sbi->s_uid))
> +			goto out;
> +	}
> +	if (zonefs_has_feature(sbi, ZONEFS_F_GID)) {
> +		sbi->s_gid = make_kgid(current_user_ns(),
> +				       le32_to_cpu(super->s_gid));
> +		if (!gid_valid(sbi->s_gid))
> +			goto out;
> +	}
> +	if (zonefs_has_feature(sbi, ZONEFS_F_PERM))
> +		sbi->s_perm = le32_to_cpu(super->s_perm);
> +
> +	ret = 0;
> +
> +out:
> +	__free_page(page);
> +
> +	return ret;
> +}
> +
> +/*
> + * Check that the device is zoned. If it is, get the list of zones and create
> + * sub-directories and files according to the device zone configuration.
> + */
> +static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
> +{
> +	struct zonefs_sb_info *sbi;
> +	struct blk_zone *zones;
> +	struct inode *inode;
> +	enum zonefs_ztype t;
> +	int ret;
> +
> +	/* Check device type */
> +	if (!bdev_is_zoned(sb->s_bdev)) {
> +		zonefs_err(sb, "Not a zoned block device\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Initialize super block information */
> +	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
> +	if (!sbi)
> +		return -ENOMEM;
> +
> +	sb->s_fs_info = sbi;
> +	sb->s_magic = ZONEFS_MAGIC;
> +	sb->s_maxbytes = MAX_LFS_FILESIZE;
> +	sb_set_blocksize(sb, bdev_physical_block_size(sb->s_bdev));
> +	sb->s_op = &zonefs_sops;
> +	sb->s_time_gran	= 1;
> +
> +	/* Set defaults */
> +	sbi->s_uid = GLOBAL_ROOT_UID;
> +	sbi->s_gid = GLOBAL_ROOT_GID;
> +	sbi->s_perm = S_IRUSR | S_IWUSR | S_IRGRP; /* 0640 */
> +
> +	ret = zonefs_read_super(sb);
> +	if (ret)
> +		return ret;
> +
> +	zones = zonefs_get_zone_info(sb);
> +	if (IS_ERR(zones))
> +		return PTR_ERR(zones);
> +
> +	pr_info("zonefs: Mounting %s, %u zones",
> +		sb->s_id, sbi->s_nr_zones[ZONEFS_ZTYPE_ALL]);
> +
> +	/* Create root directory inode */
> +	ret = -ENOMEM;
> +	inode = new_inode(sb);
> +	if (!inode)
> +		goto out;
> +	inode->i_ino = get_next_ino();
> +	inode->i_mode = S_IFDIR | 0755;
> +	inode->i_ctime = inode->i_mtime = inode->i_atime = current_time(inode);
> +	inode->i_op = &simple_dir_inode_operations;
> +	inode->i_fop = &simple_dir_operations;
> +	inode->i_size = sizeof(struct dentry) * 2;
> +	set_nlink(inode, 2);
> +	sb->s_root = d_make_root(inode);
> +	if (!sb->s_root)
> +		goto out;
> +
> +	/* Create and populate zone groups */
> +	for (t = ZONEFS_ZTYPE_CNV; t < ZONEFS_ZTYPE_MAX; t++) {
> +		ret = zonefs_create_zgroup(sb, zones, t);
> +		if (ret)
> +			break;
> +	}
> +
> +out:
> +	kvfree(zones);
> +
> +	return ret;
> +}
> +
> +static struct dentry *zonefs_mount(struct file_system_type *fs_type,
> +				 int flags, const char *dev_name, void *data)
> +{
> +	return mount_bdev(fs_type, flags, dev_name, data, zonefs_fill_super);
> +}
> +
> +static void zonefs_kill_super(struct super_block *sb)
> +{
> +	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
> +
> +	kfree(sbi);
> +	if (sb->s_root)
> +		d_genocide(sb->s_root);
> +	kill_block_super(sb);
> +}
> +
> +/*
> + * File system definition and registration.
> + */
> +static struct file_system_type zonefs_type = {
> +	.owner		= THIS_MODULE,
> +	.name		= "zonefs",
> +	.mount		= zonefs_mount,
> +	.kill_sb	= zonefs_kill_super,
> +	.fs_flags	= FS_REQUIRES_DEV,
> +};
> +
> +static int __init zonefs_init_inodecache(void)
> +{
> +	zonefs_inode_cachep = kmem_cache_create("zonefs_inode_cache",
> +			sizeof(struct zonefs_inode_info), 0,
> +			(SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD | SLAB_ACCOUNT),
> +			NULL);
> +	if (zonefs_inode_cachep == NULL)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static void zonefs_destroy_inodecache(void)
> +{
> +	/*
> +	 * Make sure all delayed rcu free inodes are flushed before we
> +	 * destroy the inode cache.
> +	 */
> +	rcu_barrier();
> +	kmem_cache_destroy(zonefs_inode_cachep);
> +}
> +
> +static int __init zonefs_init(void)
> +{
> +	int ret;
> +
> +	ret = zonefs_init_inodecache();
> +	if (ret)
> +		return ret;
> +
> +	ret = register_filesystem(&zonefs_type);
> +	if (ret) {
> +		zonefs_destroy_inodecache();
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit zonefs_exit(void)
> +{
> +	zonefs_destroy_inodecache();
> +	unregister_filesystem(&zonefs_type);
> +}
> +
> +MODULE_AUTHOR("Damien Le Moal");
> +MODULE_DESCRIPTION("Zone file system for zoned block devices");
> +MODULE_LICENSE("GPL");
> +module_init(zonefs_init);
> +module_exit(zonefs_exit);
> diff --git a/fs/zonefs/zonefs.h b/fs/zonefs/zonefs.h
> new file mode 100644
> index 000000000000..91a9081c0d77
> --- /dev/null
> +++ b/fs/zonefs/zonefs.h
> @@ -0,0 +1,190 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple zone file system for zoned block devices.
> + *
> + * Copyright (C) 2019 Western Digital Corporation or its affiliates.
> + */
> +#ifndef __ZONEFS_H__
> +#define __ZONEFS_H__
> +
> +#include <linux/fs.h>
> +#include <linux/magic.h>
> +
> +/*
> + * Maximum length of file names: this only needs to be large enough to fit
> + * the zone group directory names and a decimal value of the start sector of
> + * the zones for file names. 16 characterse is plenty.
> + */
> +#define ZONEFS_NAME_MAX		16
> +
> +/*
> + * Zone types: ZONEFS_ZTYPE_SEQWRITE is used for all sequential zone types
> + * defined in linux/blkzoned.h, that is, BLK_ZONE_TYPE_SEQWRITE_REQ and
> + * BLK_ZONE_TYPE_SEQWRITE_PREF.
> + */
> +enum zonefs_ztype {
> +	ZONEFS_ZTYPE_ALL = 0,
> +	ZONEFS_ZTYPE_CNV,
> +	ZONEFS_ZTYPE_SEQ,
> +	ZONEFS_ZTYPE_MAX,
> +};
> +
> +static inline enum zonefs_ztype zonefs_zone_type(struct blk_zone *zone)
> +{
> +	if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
> +		return ZONEFS_ZTYPE_CNV;
> +	return ZONEFS_ZTYPE_SEQ;
> +}
> +
> +static inline bool zonefs_zone_offline(struct blk_zone *zone)
> +{
> +	return zone->cond == BLK_ZONE_COND_OFFLINE;
> +}
> +
> +static inline bool zonefs_zone_readonly(struct blk_zone *zone)
> +{
> +	return zone->cond == BLK_ZONE_COND_READONLY;
> +}
> +
> +/*
> + * Inode private data.
> + */
> +struct zonefs_inode_info {
> +	struct inode		i_vnode;
> +	enum zonefs_ztype	i_ztype;
> +	loff_t			i_addr;
> +	loff_t			i_wpoffset;
> +	loff_t			i_max_size;
> +	struct rw_semaphore	i_mmap_sem;
> +};
> +
> +static inline struct zonefs_inode_info *ZONEFS_I(struct inode *inode)
> +{
> +	return container_of(inode, struct zonefs_inode_info, i_vnode);
> +}
> +
> +static inline bool zonefs_file_is_conv(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_CNV;
> +}
> +
> +static inline bool zonefs_file_is_seq(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_SEQ;
> +}
> +
> +/*
> + * Address (byte offset) on disk of a file zone.
> + */
> +static inline loff_t zonefs_file_addr(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_addr;
> +}
> +
> +/*
> + * Maximum possible size of a file (i.e. the zone size).
> + */
> +static inline loff_t zonefs_file_max_size(struct inode *inode)
> +{
> +	return ZONEFS_I(inode)->i_max_size;
> +}
> +
> +/*
> + * On-disk super block (block 0).
> + */
> +struct zonefs_super {
> +
> +	/* Magic number */
> +	__le32		s_magic;		/*    4 */
> +
> +	/* Metadata version number */
> +	__le32		s_version;		/*    8 */
> +
> +	/* Features */
> +	__le64		s_features;		/*   16 */
> +
> +	/* 128-bit uuid */
> +	__u8		s_uuid[16];		/*   32 */
> +
> +	/* UID/GID to use for files */
> +	__le32		s_uid;			/*   36 */
> +	__le32		s_gid;			/*   40 */
> +
> +	/* File permissions */
> +	__le32		s_perm;			/*   44 */
> +
> +	/* Padding to 4K */
> +	__u8		s_reserved[4052];	/* 4096 */
> +
> +} __attribute__ ((packed));
> +
> +/*
> + * Metadata version.
> + */
> +#define ZONEFS_VERSION	1
> +
> +/*
> + * Feature flags.
> + */
> +enum zonefs_features {
> +	/*
> +	 * Use a zone start sector value as file name.
> +	 */
> +	ZONEFS_F_STARTSECT_NAME,
> +	/*
> +	 * Aggregate contiguous conventional zones into a single file.
> +	 */
> +	ZONEFS_F_AGRCNV,
> +	/*
> +	 * Use super block specified UID for files instead of default.
> +	 */
> +	ZONEFS_F_UID,
> +	/*
> +	 * Use super block specified GID for files instead of default.
> +	 */
> +	ZONEFS_F_GID,
> +	/*
> +	 * Use super block specified file permissions instead of default 640.
> +	 */
> +	ZONEFS_F_PERM,
> +};
> +
> +/*
> + * In-memory Super block information.
> + */
> +struct zonefs_sb_info {
> +	/*
> +	 * Mount options.
> +	 */
> +	unsigned long		s_mnt_opt;
> +	unsigned long long	s_features;
> +	kuid_t			s_uid;		/* File owner UID */
> +	kgid_t			s_gid;		/* File owner GID */
> +	umode_t			s_perm;		/* File permissions */
> +
> +	/*
> +	 * Device information.
> +	 */
> +	loff_t			s_blocksize_mask;
> +	unsigned int		s_nr_zones[ZONEFS_ZTYPE_MAX];
> +};
> +
> +static inline struct zonefs_sb_info *ZONEFS_SB(struct super_block *sb)
> +{
> +	return sb->s_fs_info;
> +}
> +
> +static inline bool zonefs_has_feature(struct zonefs_sb_info *sbi,
> +				      enum zonefs_features f)
> +{
> +	return sbi->s_features & (1ULL << f);
> +}
> +
> +#define zonefs_info(sb, format, args...)	\
> +	pr_info("zonefs (%s): " format, sb->s_id, ## args)
> +#define zonefs_err(sb, format, args...)	\
> +	pr_err("zonefs (%s) ERROR: " format, sb->s_id, ## args)
> +#define zonefs_warn(sb, format, args...)	\
> +	pr_warn("zonefs (%s) WARN: " format, sb->s_id, ## args)
> +
> +#endif
> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> index f8c00045d537..57b627429f41 100644
> --- a/include/uapi/linux/magic.h
> +++ b/include/uapi/linux/magic.h
> @@ -86,6 +86,7 @@
>  #define NSFS_MAGIC		0x6e736673
>  #define BPF_FS_MAGIC		0xcafe4a11
>  #define AAFS_MAGIC		0x5a3c69f0
> +#define ZONEFS_MAGIC		0x5a4f4653
>  
>  /* Since UDF 2.01 is ISO 13346 based... */
>  #define UDF_SUPER_MAGIC		0x15013346
Damien Le Moal July 18, 2019, 11:02 p.m. UTC | #14
Jeff,

On 2019/07/18 23:11, Jeff Moyer wrote:
> Hi, Damien,
> 
> Did you consider creating a shared library?  I bet that would also
> ease application adoption for the use cases you're interested in, and
> would have similar performance.
> 
> -Jeff

Yes, it would, but to a lesser extent since system calls would need to be
replaced with library calls. Earlier work on LevelDB by Ting used the library
approach with libzbc, not quite a "libzonefs" but close enough. Working with
LevelDB code gave me the idea for zonefs. Compared to a library, the added
benefits are that specific language bindings are not a problem and further
simplify the code changes needed to support zoned block devices. In the case of
LevelDB for instance, C++ is used and file accesses are using streams, which
makes using a library a little difficult, and necessitates more changes just for
the internal application API itself. The needed changes spread beyond the device
access API.

This is I think the main advantage of this simple in-kernel FS over a library:
the developer can focus on zone block device specific needs (write sequential
pattern and garbage collection) and forget about the device access parts as the
standard system calls API can be used.

Another approach I considered is using FUSE, but went for a regular (albeit
simple) in-kernel approach due to performance concerns. While any difference in
performance for SMR HDDs would probably not be noticeable, performance would
likely be lower for upcoming NVMe zonenamespace devices compared to the
in-kernel approach.

But granted, most of the arguments I can put forward for an in-kernel FS
solution vs a user shared library solution are mostly subjective. I think though
that having support directly provided by the kernel brings zoned block devices
into the "mainstream storage options" rather than having them perceived as
fringe solutions that need additional libraries to work correctly. Zoned block
devices are not going away and may in fact become more mainstream as
implementing higher capacities more and more depends on the sequential write
interface.

Best regards.
Jeff Moyer July 19, 2019, 2:25 p.m. UTC | #15
Hi, Damien,

Thanks for your well-considered response.

Damien Le Moal <Damien.LeMoal@wdc.com> writes:

> Jeff,
>
> On 2019/07/18 23:11, Jeff Moyer wrote:
>> Hi, Damien,
>> 
>> Did you consider creating a shared library?  I bet that would also
>> ease application adoption for the use cases you're interested in, and
>> would have similar performance.
>> 
>> -Jeff
>
> Yes, it would, but to a lesser extent since system calls would need to be
> replaced with library calls. Earlier work on LevelDB by Ting used the library
> approach with libzbc, not quite a "libzonefs" but close enough. Working with
> LevelDB code gave me the idea for zonefs. Compared to a library, the added
> benefits are that specific language bindings are not a problem and further
> simplify the code changes needed to support zoned block devices. In the case of
> LevelDB for instance, C++ is used and file accesses are using streams, which
> makes using a library a little difficult, and necessitates more changes just for
> the internal application API itself. The needed changes spread beyond the device
> access API.
>
> This is I think the main advantage of this simple in-kernel FS over a library:
> the developer can focus on zone block device specific needs (write sequential
> pattern and garbage collection) and forget about the device access parts as the
> standard system calls API can be used.

OK, I can see how a file system eases adoption across multiple
languages, and may, in some cases, be easier to adopt by applications.
However, I'm not a fan of the file system interface for this usage.
Once you present a file system, there are certain expectations from
users, and this fs breaks most of them.

I'll throw out another suggestion that may or may not work (I haven't
given it much thought).  Would it be possible to create a device mapper
target that would export each zone as a separate block device?  I
understand that wouldn't help with the write pointer management, but it
would allow you to create a single "file" for each zone.

> Another approach I considered is using FUSE, but went for a regular (albeit
> simple) in-kernel approach due to performance concerns. While any difference in
> performance for SMR HDDs would probably not be noticeable, performance would
> likely be lower for upcoming NVMe zonenamespace devices compared to the
> in-kernel approach.
>
> But granted, most of the arguments I can put forward for an in-kernel FS
> solution vs a user shared library solution are mostly subjective. I think though
> that having support directly provided by the kernel brings zoned block devices
> into the "mainstream storage options" rather than having them perceived as
> fringe solutions that need additional libraries to work correctly. Zoned block
> devices are not going away and may in fact become more mainstream as
> implementing higher capacities more and more depends on the sequential write
> interface.

A file system like this would further cement in my mind that zoned block
devices are not maintstream storage options.  I guess this part is
highly subjective.  :)

Cheers,
Jeff
Damien Le Moal July 20, 2019, 1:07 a.m. UTC | #16
On 2019/07/19 23:25, Jeff Moyer wrote:
> Hi, Damien,
> 
> Thanks for your well-considered response.
> 
> Damien Le Moal <Damien.LeMoal@wdc.com> writes:
> 
>> Jeff,
>>
>> On 2019/07/18 23:11, Jeff Moyer wrote:
>>> Hi, Damien,
>>>
>>> Did you consider creating a shared library?  I bet that would also
>>> ease application adoption for the use cases you're interested in, and
>>> would have similar performance.
>>>
>>> -Jeff
>>
>> Yes, it would, but to a lesser extent since system calls would need to be
>> replaced with library calls. Earlier work on LevelDB by Ting used the library
>> approach with libzbc, not quite a "libzonefs" but close enough. Working with
>> LevelDB code gave me the idea for zonefs. Compared to a library, the added
>> benefits are that specific language bindings are not a problem and further
>> simplify the code changes needed to support zoned block devices. In the case of
>> LevelDB for instance, C++ is used and file accesses are using streams, which
>> makes using a library a little difficult, and necessitates more changes just for
>> the internal application API itself. The needed changes spread beyond the device
>> access API.
>>
>> This is I think the main advantage of this simple in-kernel FS over a library:
>> the developer can focus on zone block device specific needs (write sequential
>> pattern and garbage collection) and forget about the device access parts as the
>> standard system calls API can be used.
> 
> OK, I can see how a file system eases adoption across multiple
> languages, and may, in some cases, be easier to adopt by applications.
> However, I'm not a fan of the file system interface for this usage.
> Once you present a file system, there are certain expectations from
> users, and this fs breaks most of them.

Yes, that is true. zonefs differs significantly from regular file systems. But I
would argue that breaking the users expectation is OK because that would happen
only and only if the user does not understand the hardware it is dealing with. I
still get emails regularly about mkfs.ext4 not working with SMR drives :)
In other words, since kernel 4.10 and exposure of HM SRM HDDs as regular block
device files, we already are in a sense breaking legacy user expectations
regarding the devices under device files... So I am not too worried about this
point.

If zonefs makes it into the kernel, I probably will be getting more emails about
"it does not work !" until SMR drive users out there understand what they are
dealing with. We are making a serious effort with documenting everything related
to zoned block devices. See https://zonedstorage.io. zonefs, if included in the
kernel, will be part of that documentation effort. Of note is that this
documentation is external to the kernel, we still need to increase our
contribution to the kernel docs for zoned block devices. And we will.

> I'll throw out another suggestion that may or may not work (I haven't
> given it much thought).  Would it be possible to create a device mapper
> target that would export each zone as a separate block device?  I
> understand that wouldn't help with the write pointer management, but it
> would allow you to create a single "file" for each zone.

Well, I do not think you need a new device mapper for this. dm-linear supports
zoned block devices and will happily allow mapping a single zone and expose a
block device file for it. My problem with this approach is that SMR drives are
huge, and getting bigger. A 15 TB drive has 55380 zones of 256 MB. Upcoming 20
TB drives have more than 75000 zones. Using dm-linear or any per-zone device
mapper target would create a huge resources pressure as the amount of memory
alone that would be used per zone would be much higher than with a file system
and the setup would also take far longer to complete compared to zonefs mount.

>> Another approach I considered is using FUSE, but went for a regular (albeit
>> simple) in-kernel approach due to performance concerns. While any difference in
>> performance for SMR HDDs would probably not be noticeable, performance would
>> likely be lower for upcoming NVMe zonenamespace devices compared to the
>> in-kernel approach.
>>
>> But granted, most of the arguments I can put forward for an in-kernel FS
>> solution vs a user shared library solution are mostly subjective. I think though
>> that having support directly provided by the kernel brings zoned block devices
>> into the "mainstream storage options" rather than having them perceived as
>> fringe solutions that need additional libraries to work correctly. Zoned block
>> devices are not going away and may in fact become more mainstream as
>> implementing higher capacities more and more depends on the sequential write
>> interface.
> 
> A file system like this would further cement in my mind that zoned block
> devices are not maintstream storage options.  I guess this part is
> highly subjective.  :)

Yes, it is subjective :) Many (even large scale) data centers are already
switching to "all SMR" backend storage, relying on traditional block devices
(SSDs mostly) for active data. For these systems, SMR is a mainstream solution.

When saying "mainstream", I was referring more to the software needed to use
these drives rather than the drives as a solution. zonefs allows mapping the
zone sequential write constraint to a known concept: file append write. And in
this sense, I think we can consider zonefs as progress.

Ideally, I would not bother with this at all and point people to Btrtfs (work in
progress) or XFS (next in the pipeline) for using an FS on zoned block devices.
But there is definitely a tendency for many distributed applications to try to
do without any file system at all (Ceph -> Bluestore is one example). And as
mentioned before, some other use cases where a fully POSIX compliant file system
is not really necessary at all (LevelDB, RocksDB). zonefs fits in the middle
ground here between removing the normal file system and going to raw block
device. Bluestore has "bluefs" underneath rocksdb after all. One cannot really
never go directly raw block device and in most cases some level of abstraction
(space management) is needed. That is where zonefs fits.

Best regards.
Damien Le Moal July 20, 2019, 7:15 a.m. UTC | #17
Jeff,

On 2019/07/19 23:25, Jeff Moyer wrote:
> OK, I can see how a file system eases adoption across multiple
> languages, and may, in some cases, be easier to adopt by applications.
> However, I'm not a fan of the file system interface for this usage.
> Once you present a file system, there are certain expectations from
> users, and this fs breaks most of them.

Your comments got me thinking more about zonefs specifications/features and I am
now wondering if I am not pushing this too far in terms of simplicity. So here
is a new RFC/Question to chew on... While keeping as a target the concept of
"file == zone" or as close to it as possible, what do you think zonefs minimal
feature set should be ?

One idea I have since a while back now is this:
1) If a zone is unused, do not show a file for it. This means adding a dynamic
"zone allocation" code and supporting O_CREAT on open, unlink, etc. So have more
normal file system calls behave as with a normal FS.
2) Allow file names to be decided by the user instead of using a fixed names.
Again, have O_CREAT behave as expected
3) Potentially allow files to grow beyond a single zone, while keeping the space
allocation unit as a zone.

Thinking of our current LevelDB/RocksDB use cases, (1) and (2) would allow even
further simplifying the support code since with these, the SSTable file
management can essentially stay completely untouched.

(3) is not necessary for LSM-Tree type use cases since typically zones are large
and so aligning SSTables to zones the most efficient approach. However, I can
see other use cases that would benefit from (3). One example would be
Surveillance system video recording or any system dealing with high bitrate
Video. E.g. A 256 MB zone size is only 100s of high definition broadcasting (20
Mbps or so). SO managing storage space in such big chunks is OK with such use cases.

These 3 additional features would make zonefs much closer to a regular FS
behavior while keeping its IO path simple enough to be in par with fast raw
block device accesses. Additional metadata management, completely absent for
now, would be needed though. But by not allowing directories (flat namespace),
this metadata management would be reduced to an inode table and a bitmap for
zone use management. Anything beyond these features and I think we would be
better off with a regular file system.

Thoughts ?
Dave Chinner July 22, 2019, 12:04 a.m. UTC | #18
On Sat, Jul 20, 2019 at 07:15:26AM +0000, Damien Le Moal wrote:
> Jeff,
> 
> On 2019/07/19 23:25, Jeff Moyer wrote:
> > OK, I can see how a file system eases adoption across multiple
> > languages, and may, in some cases, be easier to adopt by applications.
> > However, I'm not a fan of the file system interface for this usage.
> > Once you present a file system, there are certain expectations from
> > users, and this fs breaks most of them.
> 
> Your comments got me thinking more about zonefs specifications/features and I am
> now wondering if I am not pushing this too far in terms of simplicity. So here
> is a new RFC/Question to chew on... While keeping as a target the concept of
> "file == zone" or as close to it as possible, what do you think zonefs minimal
> feature set should be ?
> 
> One idea I have since a while back now is this:
> 1) If a zone is unused, do not show a file for it. This means adding a dynamic
> "zone allocation" code and supporting O_CREAT on open, unlink, etc. So have more
> normal file system calls behave as with a normal FS.
> 2) Allow file names to be decided by the user instead of using a fixed names.
> Again, have O_CREAT behave as expected

So now you have to implement a persistent directory structure,
atomic/transactional updates, etc. You've just added at least 2
orders of magnitude complexity to zonefs and a very substantial
amount of additional, ongoing QA to ensure it works correctly.

I think keeping it simple by exposing all zones to userspace and
leaving it to the application to track/index what zones it is
using is the simplest way forward for everyone.

Cheers,

Dave.
Damien Le Moal July 22, 2019, 12:09 a.m. UTC | #19
On 2019/07/22 9:05, Dave Chinner wrote:
> On Sat, Jul 20, 2019 at 07:15:26AM +0000, Damien Le Moal wrote:
>> Jeff,
>>
>> On 2019/07/19 23:25, Jeff Moyer wrote:
>>> OK, I can see how a file system eases adoption across multiple
>>> languages, and may, in some cases, be easier to adopt by applications.
>>> However, I'm not a fan of the file system interface for this usage.
>>> Once you present a file system, there are certain expectations from
>>> users, and this fs breaks most of them.
>>
>> Your comments got me thinking more about zonefs specifications/features and I am
>> now wondering if I am not pushing this too far in terms of simplicity. So here
>> is a new RFC/Question to chew on... While keeping as a target the concept of
>> "file == zone" or as close to it as possible, what do you think zonefs minimal
>> feature set should be ?
>>
>> One idea I have since a while back now is this:
>> 1) If a zone is unused, do not show a file for it. This means adding a dynamic
>> "zone allocation" code and supporting O_CREAT on open, unlink, etc. So have more
>> normal file system calls behave as with a normal FS.
>> 2) Allow file names to be decided by the user instead of using a fixed names.
>> Again, have O_CREAT behave as expected
> 
> So now you have to implement a persistent directory structure,
> atomic/transactional updates, etc. You've just added at least 2
> orders of magnitude complexity to zonefs and a very substantial
> amount of additional, ongoing QA to ensure it works correctly.
> 
> I think keeping it simple by exposing all zones to userspace and
> leaving it to the application to track/index what zones it is
> using is the simplest way forward for everyone.

OK. Anything more complicated would probably be better implemented within an
existing file system.

Thank you for your comments.

Best regards.
Dave Chinner July 22, 2019, 12:12 a.m. UTC | #20
On Sat, Jul 20, 2019 at 01:07:25AM +0000, Damien Le Moal wrote:
> On 2019/07/19 23:25, Jeff Moyer wrote:
> > I'll throw out another suggestion that may or may not work (I haven't
> > given it much thought).  Would it be possible to create a device mapper
> > target that would export each zone as a separate block device?  I
> > understand that wouldn't help with the write pointer management, but it
> > would allow you to create a single "file" for each zone.
> 
> Well, I do not think you need a new device mapper for this. dm-linear supports
> zoned block devices and will happily allow mapping a single zone and expose a
> block device file for it. My problem with this approach is that SMR drives are
> huge, and getting bigger. A 15 TB drive has 55380 zones of 256 MB. Upcoming 20
> TB drives have more than 75000 zones. Using dm-linear or any per-zone device
> mapper target would create a huge resources pressure as the amount of memory
> alone that would be used per zone would be much higher than with a file system
> and the setup would also take far longer to complete compared to zonefs mount.

Right, it's kinda insane to expect userspace to manage tens of
thousands of "block devices" like this. You go run blkid on one of
these devices, and what happens? Then there's stuff like udev
overhead, grub os-probing that walks all block devices it can find,
etc. Then consider putting hundreds of SMR drives into a machine
that has multiple paths to each drive....

As such, I just don't think this block device approach is feasible,
especially as Managing tens of thousands of individual small data
regions in a storage device is exactly what filesystems are for.

Cheers,

Dave.

Patch
diff mbox series

diff --git a/fs/Kconfig b/fs/Kconfig
index f1046cf6ad85..e48cc0e0efbb 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -41,6 +41,7 @@  source "fs/ocfs2/Kconfig"
 source "fs/btrfs/Kconfig"
 source "fs/nilfs2/Kconfig"
 source "fs/f2fs/Kconfig"
+source "fs/zonefs/Kconfig"
 
 config FS_DAX
 	bool "Direct Access (DAX) support"
@@ -262,6 +263,7 @@  source "fs/romfs/Kconfig"
 source "fs/pstore/Kconfig"
 source "fs/sysv/Kconfig"
 source "fs/ufs/Kconfig"
+source "fs/ufs/Kconfig"
 
 endif # MISC_FILESYSTEMS
 
diff --git a/fs/Makefile b/fs/Makefile
index c9aea23aba56..02fcd528991b 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -130,3 +130,4 @@  obj-$(CONFIG_F2FS_FS)		+= f2fs/
 obj-$(CONFIG_CEPH_FS)		+= ceph/
 obj-$(CONFIG_PSTORE)		+= pstore/
 obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
+obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
diff --git a/fs/zonefs/Kconfig b/fs/zonefs/Kconfig
new file mode 100644
index 000000000000..6490547e9763
--- /dev/null
+++ b/fs/zonefs/Kconfig
@@ -0,0 +1,9 @@ 
+config ZONEFS_FS
+	tristate "zonefs filesystem support"
+	depends on BLOCK
+	depends on BLK_DEV_ZONED
+	help
+	  zonefs is a simple File System which exposes zones of a zoned block
+	  device as files.
+
+	  If unsure, say N.
diff --git a/fs/zonefs/Makefile b/fs/zonefs/Makefile
new file mode 100644
index 000000000000..75a380aa1ae1
--- /dev/null
+++ b/fs/zonefs/Makefile
@@ -0,0 +1,4 @@ 
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_ZONEFS_FS) += zonefs.o
+
+zonefs-y	:= super.o
diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
new file mode 100644
index 000000000000..0bf56f378a77
--- /dev/null
+++ b/fs/zonefs/super.c
@@ -0,0 +1,1004 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple zone file system for zoned block devices.
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ */
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/magic.h>
+#include <linux/iomap.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/blkdev.h>
+#include <linux/statfs.h>
+#include <linux/writeback.h>
+#include <linux/quotaops.h>
+#include <linux/seq_file.h>
+#include <linux/parser.h>
+#include <linux/uio.h>
+#include <linux/mman.h>
+#include <linux/sched/mm.h>
+
+#include "zonefs.h"
+
+static int zonefs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+			      unsigned int flags, struct iomap *iomap)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
+	loff_t max_isize = zonefs_file_max_size(inode);
+	loff_t isize = i_size_read(inode);
+
+	/*
+	 * For sequential zones, enforce direct IO writes. This is already
+	 * checked when writes are issued, so warn about this here if we
+	 * get buffered write to a sequential file inode.
+	 */
+	if (WARN_ON_ONCE(zonefs_file_is_seq(inode) && (flags & IOMAP_WRITE) &&
+			 (!(flags & IOMAP_DIRECT))))
+		return -EIO;
+
+	/* An IO cannot exceed the zone size */
+	if (offset >= max_isize)
+		return -EFBIG;
+
+	/* All blocks are always mapped */
+	if (offset >= i_size_read(inode)) {
+		length = min(length, max_isize - offset);
+		iomap->type = IOMAP_UNWRITTEN;
+	} else {
+		length = min(length, isize - offset);
+		iomap->type = IOMAP_MAPPED;
+	}
+	iomap->offset = offset & (~sbi->s_blocksize_mask);
+	iomap->length = (offset + length + sbi->s_blocksize_mask) &
+			(~sbi->s_blocksize_mask);
+	iomap->addr = zonefs_file_addr(inode) + iomap->offset;
+	iomap->bdev = inode->i_sb->s_bdev;
+
+	return 0;
+}
+
+static const struct iomap_ops zonefs_iomap_ops = {
+	.iomap_begin	= zonefs_iomap_begin,
+};
+
+static int zonefs_readpage(struct file *unused, struct page *page)
+{
+	return iomap_readpage(page, &zonefs_iomap_ops);
+}
+
+static int zonefs_readpages(struct file *unused, struct address_space *mapping,
+			    struct list_head *pages, unsigned int nr_pages)
+{
+	return iomap_readpages(mapping, pages, nr_pages, &zonefs_iomap_ops);
+}
+
+static int zonefs_map_blocks(struct iomap_writepage_ctx *wpc,
+			     struct inode *inode, loff_t offset)
+{
+	if (offset >= wpc->iomap.offset &&
+	    offset < wpc->iomap.offset + wpc->iomap.length)
+		return 0;
+
+	memset(&wpc->iomap, 0, sizeof(wpc->iomap));
+	return zonefs_iomap_begin(inode, offset, INT_MAX, 0, &wpc->iomap);
+}
+
+static const struct iomap_writeback_ops zonefs_writeback_ops = {
+	.map_blocks		= zonefs_map_blocks,
+};
+
+static int zonefs_writepage(struct page *page, struct writeback_control *wbc)
+{
+	struct iomap_writepage_ctx wpc = { };
+
+	return iomap_writepage(page, wbc, &wpc, &zonefs_writeback_ops);
+}
+
+static int zonefs_writepages(struct address_space *mapping,
+			     struct writeback_control *wbc)
+{
+	struct iomap_writepage_ctx wpc = { };
+
+	return iomap_writepages(mapping, wbc, &wpc, &zonefs_writeback_ops);
+}
+
+static const struct address_space_operations zonefs_file_aops = {
+	.readpage		= zonefs_readpage,
+	.readpages		= zonefs_readpages,
+	.writepage		= zonefs_writepage,
+	.writepages		= zonefs_writepages,
+	.set_page_dirty		= iomap_set_page_dirty,
+	.releasepage		= iomap_releasepage,
+	.invalidatepage		= iomap_invalidatepage,
+	.migratepage		= iomap_migrate_page,
+	.is_partially_uptodate  = iomap_is_partially_uptodate,
+	.error_remove_page	= generic_error_remove_page,
+	.direct_IO		= noop_direct_IO,
+};
+
+static int zonefs_truncate_seqfile(struct inode *inode)
+{
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	int ret;
+
+	/* Serialize against page faults */
+	down_write(&zi->i_mmap_sem);
+
+	ret = blkdev_reset_zones(inode->i_sb->s_bdev,
+				 zonefs_file_addr(inode) >> SECTOR_SHIFT,
+				 zonefs_file_max_size(inode) >> SECTOR_SHIFT,
+				 GFP_KERNEL);
+	if (ret) {
+		zonefs_err(inode->i_sb,
+			   "zonefs: Reset zone at %llu failed %d",
+			   zonefs_file_addr(inode) >> SECTOR_SHIFT,
+			   ret);
+	} else {
+		truncate_setsize(inode, 0);
+		zi->i_wpoffset = 0;
+	}
+
+	up_write(&zi->i_mmap_sem);
+
+	return ret;
+}
+
+static int zonefs_inode_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+	struct inode *inode = d_inode(dentry);
+	int ret;
+
+	ret = setattr_prepare(dentry, iattr);
+	if (ret)
+		return ret;
+
+	if ((iattr->ia_valid & ATTR_UID &&
+	     !uid_eq(iattr->ia_uid, inode->i_uid)) ||
+	    (iattr->ia_valid & ATTR_GID &&
+	     !gid_eq(iattr->ia_gid, inode->i_gid))) {
+		ret = dquot_transfer(inode, iattr);
+		if (ret)
+			return ret;
+	}
+
+	if (iattr->ia_valid & ATTR_SIZE) {
+		/* The size of conventional zone files cannot be changed */
+		if (zonefs_file_is_conv(inode))
+			return -EPERM;
+
+		/*
+		 * For sequential zone files, we can only allow truncating to
+		 * 0 size which is equivalent to a zone reset.
+		 */
+		if (iattr->ia_size != 0)
+			return -EPERM;
+
+		ret = zonefs_truncate_seqfile(inode);
+		if (ret)
+			return ret;
+	}
+
+	setattr_copy(inode, iattr);
+
+	return 0;
+}
+
+static const struct inode_operations zonefs_file_inode_operations = {
+	.setattr	= zonefs_inode_setattr,
+};
+
+/*
+ * Open a file.
+ */
+static int zonefs_file_open(struct inode *inode, struct file *file)
+{
+	/*
+	 * Note: here we can do an explicit open of the file zone,
+	 * on the first open of the inode. The explicit close can be
+	 * done on the last release (close) call for the inode.
+	 */
+
+	return generic_file_open(inode, file);
+}
+
+static int zonefs_file_fsync(struct file *file, loff_t start, loff_t end,
+			     int datasync)
+{
+	struct inode *inode = file_inode(file);
+	int ret;
+
+	/*
+	 * Since only direct writes are allowed in sequential files, we only
+	 * need a device flush for these files.
+	 */
+	if (zonefs_file_is_seq(inode))
+		goto flush;
+
+	ret = file_write_and_wait_range(file, start, end);
+	if (ret == 0)
+		ret = file_check_and_advance_wb_err(file);
+	if (ret)
+		return ret;
+
+flush:
+	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
+}
+
+static vm_fault_t zonefs_filemap_fault(struct vm_fault *vmf)
+{
+	struct zonefs_inode_info *zi = ZONEFS_I(file_inode(vmf->vma->vm_file));
+	vm_fault_t ret;
+
+	down_read(&zi->i_mmap_sem);
+	ret = filemap_fault(vmf);
+	up_read(&zi->i_mmap_sem);
+
+	return ret;
+}
+
+static vm_fault_t zonefs_filemap_page_mkwrite(struct vm_fault *vmf)
+{
+	struct inode *inode = file_inode(vmf->vma->vm_file);
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	vm_fault_t ret;
+
+	sb_start_pagefault(inode->i_sb);
+	file_update_time(vmf->vma->vm_file);
+
+	/* Serialize against truncates */
+	down_read(&zi->i_mmap_sem);
+	ret = iomap_page_mkwrite(vmf, &zonefs_iomap_ops);
+	up_read(&zi->i_mmap_sem);
+
+	sb_end_pagefault(inode->i_sb);
+	return ret;
+}
+
+static const struct vm_operations_struct zonefs_file_vm_ops = {
+	.fault		= zonefs_filemap_fault,
+	.map_pages	= filemap_map_pages,
+	.page_mkwrite	= zonefs_filemap_page_mkwrite,
+};
+
+static int zonefs_file_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	/*
+	 * Conventional zone files can be mmap-ed READ/WRITE.
+	 * For sequential zone files, only readonly mappings are possible.
+	 */
+	if (zonefs_file_is_seq(file_inode(file)) &&
+	    (vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
+		return -EINVAL;
+
+	file_accessed(file);
+	vma->vm_ops = &zonefs_file_vm_ops;
+
+	return 0;
+}
+
+static loff_t zonefs_file_llseek(struct file *file, loff_t offset, int whence)
+{
+	loff_t isize = i_size_read(file_inode(file));
+
+	/*
+	 * Seeks are limited to below the zone size for conventional zones
+	 * and below the zone write pointer for sequential zones. In both
+	 * cases, this limit is the inode size.
+	 */
+	return generic_file_llseek_size(file, offset, whence, isize, isize);
+}
+
+static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
+	loff_t max_pos = zonefs_file_max_size(inode);
+	size_t count;
+	ssize_t ret = 0;
+
+	/*
+	 * Check that the read operation does not go beyond the maximum
+	 * file size.
+	 */
+	if (iocb->ki_pos >= zonefs_file_max_size(inode))
+		return -EFBIG;
+
+	/*
+	 * For sequential zones, limit reads to written data.
+	 */
+	if (zonefs_file_is_seq(inode))
+		max_pos = i_size_read(inode);
+	if (iocb->ki_pos >= max_pos)
+		return 0;
+
+	iov_iter_truncate(to, max_pos - iocb->ki_pos);
+	count = iov_iter_count(to);
+	if (!count)
+		return 0;
+
+	/* Direct IO reads must be aligned to device physical sector size */
+	if ((iocb->ki_flags & IOCB_DIRECT) &&
+	    ((iocb->ki_pos | count) & sbi->s_blocksize_mask))
+		return -EINVAL;
+
+	if (iocb->ki_flags & IOCB_NOWAIT) {
+		if (!inode_trylock_shared(inode))
+			return -EAGAIN;
+	} else {
+		inode_lock_shared(inode);
+	}
+
+	if (iocb->ki_flags & IOCB_DIRECT) {
+		file_accessed(iocb->ki_filp);
+		ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, NULL);
+	} else {
+		ret = generic_file_read_iter(iocb, to);
+	}
+
+	inode_unlock_shared(inode);
+
+	return ret;
+}
+
+/*
+ * We got a write error: get the sequenial zone information from the device to
+ * figure out where the zone write pointer is and verify the inode size against
+ * it.
+ */
+static int zonefs_write_failed(struct inode *inode, int error)
+{
+	struct super_block *sb = inode->i_sb;
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	sector_t sector = zi->i_addr >> SECTOR_SHIFT;
+	unsigned int noio_flag;
+	struct blk_zone zone;
+	int n = 1, ret;
+
+	zonefs_warn(sb, "Updating inode zone %llu info\n", sector);
+
+	noio_flag = memalloc_noio_save();
+	ret = blkdev_report_zones(sb->s_bdev, sector, &zone, &n);
+	memalloc_noio_restore(noio_flag);
+
+	if (!n)
+		ret = -EIO;
+	if (ret) {
+		zonefs_err(sb, "Get zone %llu report failed %d\n",
+			   sector, ret);
+		return ret;
+	}
+
+	zi->i_wpoffset = (zone.wp - zone.start) << SECTOR_SHIFT;
+	if (i_size_read(inode) != zi->i_wpoffset) {
+		i_size_write(inode, zi->i_wpoffset);
+		truncate_pagecache(inode, zi->i_wpoffset);
+	}
+
+	return error;
+}
+
+static int zonefs_update_size(struct inode *inode, loff_t new_pos)
+{
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+
+	zi->i_wpoffset = new_pos;
+	if (new_pos > i_size_read(inode))
+		i_size_write(inode, new_pos);
+	return 0;
+}
+
+static int zonefs_dio_seqwrite_end_io(struct kiocb *iocb, ssize_t size,
+				      unsigned int flags)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	int ret;
+
+	inode_lock(inode);
+	if (size < 0)
+		ret = zonefs_write_failed(inode, size);
+	else
+		ret = zonefs_update_size(inode, iocb->ki_pos + size);
+	inode_unlock(inode);
+	return ret;
+}
+
+static ssize_t zonefs_file_dio_aio_write(struct kiocb *iocb,
+					 struct iov_iter *from)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	size_t count;
+
+	/*
+	 * The size of conventional zone files is fixed to the zone size.
+	 * So only direct writes to sequential zones need adjusting the
+	 * inode size on IO completion.
+	 */
+	if (zonefs_file_is_conv(inode))
+		return iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
+
+	/* Enforce append only sequential writes */
+	count = iov_iter_count(from);
+	if (iocb->ki_pos != zi->i_wpoffset) {
+		zonefs_err(inode->i_sb,
+			   "Unaligned write at %llu + %zu (wp %llu)\n",
+			   iocb->ki_pos, count, zi->i_wpoffset);
+		return -EINVAL;
+	}
+
+	if (is_sync_kiocb(iocb)) {
+		/*
+		 * Don't use the end_io callback for synchronous iocbs,
+		 * as we'd deadlock on i_rwsem.  Instead perform the same
+		 * actions manually here.
+		 */
+		count = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, NULL);
+		if (count < 0)
+			return zonefs_write_failed(inode, count);
+		zonefs_update_size(inode, iocb->ki_pos);
+		return count;
+	}
+
+	return iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
+			    zonefs_dio_seqwrite_end_io);
+}
+
+static ssize_t zonefs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct zonefs_sb_info *sbi = ZONEFS_SB(inode->i_sb);
+	size_t count;
+	ssize_t ret;
+
+	/*
+	 * Check that the read operation does not go beyond the file
+	 * zone boundary.
+	 */
+	if (iocb->ki_pos >= zonefs_file_max_size(inode))
+		return -EFBIG;
+	iov_iter_truncate(from, zonefs_file_max_size(inode) - iocb->ki_pos);
+	count = iov_iter_count(from);
+
+	if (!count)
+		return 0;
+
+	/*
+	 * Direct IO writes are mandatory for sequential zones so that write IO
+	 * order is preserved. The direct writes also must be aligned to
+	 * device physical sector size.
+	 */
+	if (iocb->ki_flags & IOCB_DIRECT) {
+		if ((iocb->ki_pos | count) & sbi->s_blocksize_mask)
+			return -EINVAL;
+	} else {
+		if (zonefs_file_is_seq(inode))
+			return -EOPNOTSUPP;
+	}
+
+	if (iocb->ki_flags & IOCB_NOWAIT) {
+		if (!inode_trylock(inode))
+			return -EAGAIN;
+	} else {
+		inode_lock(inode);
+	}
+
+	ret = generic_write_checks(iocb, from);
+	if (ret <= 0)
+		goto out;
+
+	if (iocb->ki_flags & IOCB_DIRECT)
+		ret = zonefs_file_dio_aio_write(iocb, from);
+	else
+		ret = iomap_file_buffered_write(iocb, from, &zonefs_iomap_ops);
+
+out:
+	inode_unlock(inode);
+
+	if (ret > 0 && (!(iocb->ki_flags & IOCB_DIRECT))) {
+		iocb->ki_pos += ret;
+		ret = generic_write_sync(iocb, ret);
+	}
+
+	return ret;
+}
+
+static const struct file_operations zonefs_file_operations = {
+	.open		= zonefs_file_open,
+	.fsync		= zonefs_file_fsync,
+	.mmap		= zonefs_file_mmap,
+	.llseek		= zonefs_file_llseek,
+	.read_iter	= zonefs_file_read_iter,
+	.write_iter	= zonefs_file_write_iter,
+	.splice_read	= generic_file_splice_read,
+	.splice_write	= iter_file_splice_write,
+	.iopoll		= iomap_dio_iopoll,
+};
+
+
+static struct kmem_cache *zonefs_inode_cachep;
+
+static struct inode *zonefs_alloc_inode(struct super_block *sb)
+{
+	struct zonefs_inode_info *zi;
+
+	zi = kmem_cache_alloc(zonefs_inode_cachep, GFP_KERNEL);
+	if (!zi)
+		return NULL;
+
+	init_rwsem(&zi->i_mmap_sem);
+	inode_init_once(&zi->i_vnode);
+
+	return &zi->i_vnode;
+}
+
+static void zonefs_destroy_cb(struct rcu_head *head)
+{
+	struct inode *inode = container_of(head, struct inode, i_rcu);
+
+	kmem_cache_free(zonefs_inode_cachep, ZONEFS_I(inode));
+}
+
+static void zonefs_destroy_inode(struct inode *inode)
+{
+	call_rcu(&inode->i_rcu, zonefs_destroy_cb);
+}
+
+static struct dentry *zonefs_create_inode(struct dentry *parent,
+					  const char *name,
+					  struct blk_zone *zone)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(parent->d_sb);
+	struct inode *dir = d_inode(parent);
+	struct dentry *dentry;
+	struct inode *inode;
+
+	dentry = d_alloc_name(parent, name);
+	if (!dentry)
+		return NULL;
+
+	inode = new_inode(parent->d_sb);
+	if (!inode)
+		goto out_dput;
+
+	inode->i_ino = get_next_ino();
+	if (zone) {
+		struct zonefs_inode_info *zi = ZONEFS_I(inode);
+
+		/* Zone file */
+		inode->i_mode = S_IFREG | sbi->s_perm;
+		if (zonefs_zone_readonly(zone))
+			inode->i_mode &= ~S_IWUGO;
+		inode->i_uid = sbi->s_uid;
+		inode->i_gid = sbi->s_gid;
+		zi->i_ztype = zonefs_zone_type(zone);
+		zi->i_addr = zone->start << SECTOR_SHIFT;
+		zi->i_max_size = zone->len << SECTOR_SHIFT;
+		inode->i_blocks = zone->len;
+		if (zonefs_file_is_conv(inode))
+			zi->i_wpoffset = zi->i_max_size;
+		else
+			zi->i_wpoffset =
+				(zone->wp - zone->start) << SECTOR_SHIFT;
+		inode->i_size = zi->i_wpoffset;
+		inode->i_fop = &zonefs_file_operations;
+		inode->i_op = &zonefs_file_inode_operations;
+		inode->i_mapping->a_ops = &zonefs_file_aops;
+		inode->i_mapping->a_ops = &zonefs_file_aops;
+	} else {
+		/* Zone group directory */
+		inode_init_owner(inode, dir, S_IFDIR | 0555);
+		inode->i_fop = &simple_dir_operations;
+		inode->i_op = &simple_dir_inode_operations;
+		set_nlink(inode, 2);
+		inc_nlink(dir);
+	}
+	inode->i_ctime = inode->i_mtime = inode->i_atime = dir->i_ctime;
+
+	d_add(dentry, inode);
+	d_inode(parent)->i_size += sizeof(struct dentry);
+
+	return dentry;
+
+out_dput:
+	dput(dentry);
+	return NULL;
+}
+
+/*
+ * File system stat.
+ */
+static int zonefs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+	struct super_block *sb = dentry->d_sb;
+	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
+	sector_t nr_sectors = sb->s_bdev->bd_part->nr_sects;
+	enum zonefs_ztype t;
+
+	buf->f_type = ZONEFS_MAGIC;
+	buf->f_bsize = dentry->d_sb->s_blocksize;
+	buf->f_namelen = ZONEFS_NAME_MAX;
+
+	buf->f_blocks = nr_sectors >> (sb->s_blocksize_bits - SECTOR_SHIFT);
+	buf->f_bfree = 0;
+	buf->f_bavail = 0;
+
+	buf->f_files = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - 1;
+	for (t = ZONEFS_ZTYPE_ALL; t < ZONEFS_ZTYPE_MAX; t++) {
+		if (sbi->s_nr_zones[t])
+			buf->f_files++;
+	}
+	buf->f_ffree = 0;
+
+	/* buf->f_fsid = 0; uuid, see ext2 */
+	buf->f_namelen = ZONEFS_NAME_MAX;
+
+	return 0;
+}
+
+static const struct super_operations zonefs_sops = {
+	.alloc_inode	= zonefs_alloc_inode,
+	.destroy_inode	= zonefs_destroy_inode,
+	.statfs		= zonefs_statfs,
+};
+
+static char *zgroups_name[ZONEFS_ZTYPE_MAX] = {
+	NULL,
+	"cnv",
+	"seq"
+};
+
+/*
+ * Create a zone group and populate it with zone files.
+ */
+static int zonefs_create_zgroup(struct super_block *sb, struct blk_zone *zones,
+				enum zonefs_ztype type)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
+	struct blk_zone *zone, *next, *end;
+	char name[ZONEFS_NAME_MAX];
+	unsigned int nr_files = 0;
+	struct dentry *dir;
+
+	/* If the group is empty, nothing to do */
+	if (!sbi->s_nr_zones[type])
+		return 0;
+
+	dir = zonefs_create_inode(sb->s_root, zgroups_name[type], NULL);
+	if (!dir)
+		return -ENOMEM;
+
+	/*
+	 * Note: The first zone contains the super block: skip it.
+	 */
+	end = zones + sbi->s_nr_zones[ZONEFS_ZTYPE_ALL];
+	for (zone = &zones[1]; zone < end; zone = next) {
+
+		next = zone + 1;
+		if (zonefs_zone_type(zone) != type)
+			continue;
+
+		/* Ignore offline zones */
+		if (zonefs_zone_offline(zone))
+			continue;
+
+		/*
+		 * For conventional zones, contiguous zones can be aggregated
+		 * together to form larger files.
+		 * Note that this overwrites the length of the first zone of
+		 * the set of contiguous zones aggregated together.
+		 * Only zones with the same condition can be agreggated so that
+		 * offline zones are excluded and readonly zones are aggregated
+		 * together into a read only file.
+		 */
+		if (type == ZONEFS_ZTYPE_CNV &&
+		    zonefs_has_feature(sbi, ZONEFS_F_AGRCNV)) {
+			for (; next < end; next++) {
+				if (zonefs_zone_type(next) != type ||
+				    next->cond != zone->cond)
+					break;
+				zone->len += next->len;
+			}
+		}
+
+		if (zonefs_has_feature(sbi, ZONEFS_F_STARTSECT_NAME))
+			/* Use zone start sector as file names */
+			snprintf(name, ZONEFS_NAME_MAX - 1, "%llu",
+				 zone->start);
+		else
+			/* Use file number as file names */
+			snprintf(name, ZONEFS_NAME_MAX - 1, "%u", nr_files);
+		nr_files++;
+
+		if (!zonefs_create_inode(dir, name, zone))
+			return -ENOMEM;
+	}
+
+	zonefs_info(sb, "Zone group %d (%s), %u zones -> %u file%s\n",
+		    type, zgroups_name[type], sbi->s_nr_zones[type],
+		    nr_files, nr_files > 1 ? "s" : "");
+
+	return 0;
+}
+
+static struct blk_zone *zonefs_get_zone_info(struct super_block *sb)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
+	struct block_device *bdev = sb->s_bdev;
+	sector_t nr_sectors = bdev->bd_part->nr_sects;
+	unsigned int i, n, nr_zones = 0;
+	struct blk_zone *zones, *zone;
+	sector_t sector = 0;
+	int ret;
+
+	sbi->s_blocksize_mask = sb->s_blocksize - 1;
+	sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] = blkdev_nr_zones(bdev);
+	zones = kvcalloc(sbi->s_nr_zones[ZONEFS_ZTYPE_ALL],
+			 sizeof(struct blk_zone), GFP_KERNEL);
+	if (!zones)
+		return ERR_PTR(-ENOMEM);
+
+	/* Get zones information */
+	zone = zones;
+	while (nr_zones < sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] &&
+	       sector < nr_sectors) {
+
+		n = sbi->s_nr_zones[ZONEFS_ZTYPE_ALL] - nr_zones;
+		ret = blkdev_report_zones(bdev, sector, zone, &n);
+		if (ret) {
+			zonefs_err(sb, "Zone report failed %d\n", ret);
+			goto err;
+		}
+		if (!n) {
+			zonefs_err(sb, "No zones reported\n");
+			ret = -EIO;
+			goto err;
+		}
+
+		for (i = 0; i < n; i++) {
+			switch (zone->type) {
+			case BLK_ZONE_TYPE_CONVENTIONAL:
+				zone->wp = zone->start + zone->len;
+				if (zone > zones)
+					sbi->s_nr_zones[ZONEFS_ZTYPE_CNV]++;
+				break;
+			case BLK_ZONE_TYPE_SEQWRITE_REQ:
+			case BLK_ZONE_TYPE_SEQWRITE_PREF:
+				if (zone > zones)
+					sbi->s_nr_zones[ZONEFS_ZTYPE_SEQ]++;
+				break;
+			default:
+				zonefs_err(sb, "Unsupported zone type 0x%x\n",
+					   zone->type);
+				ret = -EIO;
+				goto err;
+			}
+			sector += zone->len;
+			zone++;
+		}
+
+		nr_zones += n;
+	}
+
+	if (sector < nr_sectors ||
+	    nr_zones < sbi->s_nr_zones[ZONEFS_ZTYPE_ALL]) {
+		zonefs_err(sb, "Incomplete zone report\n");
+		ret = -EIO;
+		goto err;
+	}
+
+	return zones;
+
+err:
+	kvfree(zones);
+	return ERR_PTR(ret);
+}
+
+/*
+ * Read super block information from the device.
+ */
+static int zonefs_read_super(struct super_block *sb)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
+	struct zonefs_super *super;
+	struct bio bio;
+	struct bio_vec bio_vec;
+	struct page *page;
+	int ret;
+
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
+
+	bio_init(&bio, &bio_vec, 1);
+	bio.bi_iter.bi_sector = 0;
+	bio_set_dev(&bio, sb->s_bdev);
+	bio_set_op_attrs(&bio, REQ_OP_READ, 0);
+	bio_add_page(&bio, page, PAGE_SIZE, 0);
+
+	ret = submit_bio_wait(&bio);
+	if (ret)
+		goto out;
+
+	ret = -EINVAL;
+	super = page_address(page);
+	if (le32_to_cpu(super->s_magic) != ZONEFS_MAGIC)
+		goto out;
+	if (le32_to_cpu(super->s_version) != ZONEFS_VERSION)
+		goto out;
+	sbi->s_features = le64_to_cpu(super->s_features);
+	if (zonefs_has_feature(sbi, ZONEFS_F_UID)) {
+		sbi->s_uid = make_kuid(current_user_ns(),
+				       le32_to_cpu(super->s_uid));
+		if (!uid_valid(sbi->s_uid))
+			goto out;
+	}
+	if (zonefs_has_feature(sbi, ZONEFS_F_GID)) {
+		sbi->s_gid = make_kgid(current_user_ns(),
+				       le32_to_cpu(super->s_gid));
+		if (!gid_valid(sbi->s_gid))
+			goto out;
+	}
+	if (zonefs_has_feature(sbi, ZONEFS_F_PERM))
+		sbi->s_perm = le32_to_cpu(super->s_perm);
+
+	ret = 0;
+
+out:
+	__free_page(page);
+
+	return ret;
+}
+
+/*
+ * Check that the device is zoned. If it is, get the list of zones and create
+ * sub-directories and files according to the device zone configuration.
+ */
+static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
+{
+	struct zonefs_sb_info *sbi;
+	struct blk_zone *zones;
+	struct inode *inode;
+	enum zonefs_ztype t;
+	int ret;
+
+	/* Check device type */
+	if (!bdev_is_zoned(sb->s_bdev)) {
+		zonefs_err(sb, "Not a zoned block device\n");
+		return -EINVAL;
+	}
+
+	/* Initialize super block information */
+	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+
+	sb->s_fs_info = sbi;
+	sb->s_magic = ZONEFS_MAGIC;
+	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	sb_set_blocksize(sb, bdev_physical_block_size(sb->s_bdev));
+	sb->s_op = &zonefs_sops;
+	sb->s_time_gran	= 1;
+
+	/* Set defaults */
+	sbi->s_uid = GLOBAL_ROOT_UID;
+	sbi->s_gid = GLOBAL_ROOT_GID;
+	sbi->s_perm = S_IRUSR | S_IWUSR | S_IRGRP; /* 0640 */
+
+	ret = zonefs_read_super(sb);
+	if (ret)
+		return ret;
+
+	zones = zonefs_get_zone_info(sb);
+	if (IS_ERR(zones))
+		return PTR_ERR(zones);
+
+	pr_info("zonefs: Mounting %s, %u zones",
+		sb->s_id, sbi->s_nr_zones[ZONEFS_ZTYPE_ALL]);
+
+	/* Create root directory inode */
+	ret = -ENOMEM;
+	inode = new_inode(sb);
+	if (!inode)
+		goto out;
+	inode->i_ino = get_next_ino();
+	inode->i_mode = S_IFDIR | 0755;
+	inode->i_ctime = inode->i_mtime = inode->i_atime = current_time(inode);
+	inode->i_op = &simple_dir_inode_operations;
+	inode->i_fop = &simple_dir_operations;
+	inode->i_size = sizeof(struct dentry) * 2;
+	set_nlink(inode, 2);
+	sb->s_root = d_make_root(inode);
+	if (!sb->s_root)
+		goto out;
+
+	/* Create and populate zone groups */
+	for (t = ZONEFS_ZTYPE_CNV; t < ZONEFS_ZTYPE_MAX; t++) {
+		ret = zonefs_create_zgroup(sb, zones, t);
+		if (ret)
+			break;
+	}
+
+out:
+	kvfree(zones);
+
+	return ret;
+}
+
+static struct dentry *zonefs_mount(struct file_system_type *fs_type,
+				 int flags, const char *dev_name, void *data)
+{
+	return mount_bdev(fs_type, flags, dev_name, data, zonefs_fill_super);
+}
+
+static void zonefs_kill_super(struct super_block *sb)
+{
+	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
+
+	kfree(sbi);
+	if (sb->s_root)
+		d_genocide(sb->s_root);
+	kill_block_super(sb);
+}
+
+/*
+ * File system definition and registration.
+ */
+static struct file_system_type zonefs_type = {
+	.owner		= THIS_MODULE,
+	.name		= "zonefs",
+	.mount		= zonefs_mount,
+	.kill_sb	= zonefs_kill_super,
+	.fs_flags	= FS_REQUIRES_DEV,
+};
+
+static int __init zonefs_init_inodecache(void)
+{
+	zonefs_inode_cachep = kmem_cache_create("zonefs_inode_cache",
+			sizeof(struct zonefs_inode_info), 0,
+			(SLAB_RECLAIM_ACCOUNT | SLAB_MEM_SPREAD | SLAB_ACCOUNT),
+			NULL);
+	if (zonefs_inode_cachep == NULL)
+		return -ENOMEM;
+	return 0;
+}
+
+static void zonefs_destroy_inodecache(void)
+{
+	/*
+	 * Make sure all delayed rcu free inodes are flushed before we
+	 * destroy the inode cache.
+	 */
+	rcu_barrier();
+	kmem_cache_destroy(zonefs_inode_cachep);
+}
+
+static int __init zonefs_init(void)
+{
+	int ret;
+
+	ret = zonefs_init_inodecache();
+	if (ret)
+		return ret;
+
+	ret = register_filesystem(&zonefs_type);
+	if (ret) {
+		zonefs_destroy_inodecache();
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit zonefs_exit(void)
+{
+	zonefs_destroy_inodecache();
+	unregister_filesystem(&zonefs_type);
+}
+
+MODULE_AUTHOR("Damien Le Moal");
+MODULE_DESCRIPTION("Zone file system for zoned block devices");
+MODULE_LICENSE("GPL");
+module_init(zonefs_init);
+module_exit(zonefs_exit);
diff --git a/fs/zonefs/zonefs.h b/fs/zonefs/zonefs.h
new file mode 100644
index 000000000000..91a9081c0d77
--- /dev/null
+++ b/fs/zonefs/zonefs.h
@@ -0,0 +1,190 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple zone file system for zoned block devices.
+ *
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ */
+#ifndef __ZONEFS_H__
+#define __ZONEFS_H__
+
+#include <linux/fs.h>
+#include <linux/magic.h>
+
+/*
+ * Maximum length of file names: this only needs to be large enough to fit
+ * the zone group directory names and a decimal value of the start sector of
+ * the zones for file names. 16 characterse is plenty.
+ */
+#define ZONEFS_NAME_MAX		16
+
+/*
+ * Zone types: ZONEFS_ZTYPE_SEQWRITE is used for all sequential zone types
+ * defined in linux/blkzoned.h, that is, BLK_ZONE_TYPE_SEQWRITE_REQ and
+ * BLK_ZONE_TYPE_SEQWRITE_PREF.
+ */
+enum zonefs_ztype {
+	ZONEFS_ZTYPE_ALL = 0,
+	ZONEFS_ZTYPE_CNV,
+	ZONEFS_ZTYPE_SEQ,
+	ZONEFS_ZTYPE_MAX,
+};
+
+static inline enum zonefs_ztype zonefs_zone_type(struct blk_zone *zone)
+{
+	if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
+		return ZONEFS_ZTYPE_CNV;
+	return ZONEFS_ZTYPE_SEQ;
+}
+
+static inline bool zonefs_zone_offline(struct blk_zone *zone)
+{
+	return zone->cond == BLK_ZONE_COND_OFFLINE;
+}
+
+static inline bool zonefs_zone_readonly(struct blk_zone *zone)
+{
+	return zone->cond == BLK_ZONE_COND_READONLY;
+}
+
+/*
+ * Inode private data.
+ */
+struct zonefs_inode_info {
+	struct inode		i_vnode;
+	enum zonefs_ztype	i_ztype;
+	loff_t			i_addr;
+	loff_t			i_wpoffset;
+	loff_t			i_max_size;
+	struct rw_semaphore	i_mmap_sem;
+};
+
+static inline struct zonefs_inode_info *ZONEFS_I(struct inode *inode)
+{
+	return container_of(inode, struct zonefs_inode_info, i_vnode);
+}
+
+static inline bool zonefs_file_is_conv(struct inode *inode)
+{
+	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_CNV;
+}
+
+static inline bool zonefs_file_is_seq(struct inode *inode)
+{
+	return ZONEFS_I(inode)->i_ztype == ZONEFS_ZTYPE_SEQ;
+}
+
+/*
+ * Address (byte offset) on disk of a file zone.
+ */
+static inline loff_t zonefs_file_addr(struct inode *inode)
+{
+	return ZONEFS_I(inode)->i_addr;
+}
+
+/*
+ * Maximum possible size of a file (i.e. the zone size).
+ */
+static inline loff_t zonefs_file_max_size(struct inode *inode)
+{
+	return ZONEFS_I(inode)->i_max_size;
+}
+
+/*
+ * On-disk super block (block 0).
+ */
+struct zonefs_super {
+
+	/* Magic number */
+	__le32		s_magic;		/*    4 */
+
+	/* Metadata version number */
+	__le32		s_version;		/*    8 */
+
+	/* Features */
+	__le64		s_features;		/*   16 */
+
+	/* 128-bit uuid */
+	__u8		s_uuid[16];		/*   32 */
+
+	/* UID/GID to use for files */
+	__le32		s_uid;			/*   36 */
+	__le32		s_gid;			/*   40 */
+
+	/* File permissions */
+	__le32		s_perm;			/*   44 */
+
+	/* Padding to 4K */
+	__u8		s_reserved[4052];	/* 4096 */
+
+} __attribute__ ((packed));
+
+/*
+ * Metadata version.
+ */
+#define ZONEFS_VERSION	1
+
+/*
+ * Feature flags.
+ */
+enum zonefs_features {
+	/*
+	 * Use a zone start sector value as file name.
+	 */
+	ZONEFS_F_STARTSECT_NAME,
+	/*
+	 * Aggregate contiguous conventional zones into a single file.
+	 */
+	ZONEFS_F_AGRCNV,
+	/*
+	 * Use super block specified UID for files instead of default.
+	 */
+	ZONEFS_F_UID,
+	/*
+	 * Use super block specified GID for files instead of default.
+	 */
+	ZONEFS_F_GID,
+	/*
+	 * Use super block specified file permissions instead of default 640.
+	 */
+	ZONEFS_F_PERM,
+};
+
+/*
+ * In-memory Super block information.
+ */
+struct zonefs_sb_info {
+	/*
+	 * Mount options.
+	 */
+	unsigned long		s_mnt_opt;
+	unsigned long long	s_features;
+	kuid_t			s_uid;		/* File owner UID */
+	kgid_t			s_gid;		/* File owner GID */
+	umode_t			s_perm;		/* File permissions */
+
+	/*
+	 * Device information.
+	 */
+	loff_t			s_blocksize_mask;
+	unsigned int		s_nr_zones[ZONEFS_ZTYPE_MAX];
+};
+
+static inline struct zonefs_sb_info *ZONEFS_SB(struct super_block *sb)
+{
+	return sb->s_fs_info;
+}
+
+static inline bool zonefs_has_feature(struct zonefs_sb_info *sbi,
+				      enum zonefs_features f)
+{
+	return sbi->s_features & (1ULL << f);
+}
+
+#define zonefs_info(sb, format, args...)	\
+	pr_info("zonefs (%s): " format, sb->s_id, ## args)
+#define zonefs_err(sb, format, args...)	\
+	pr_err("zonefs (%s) ERROR: " format, sb->s_id, ## args)
+#define zonefs_warn(sb, format, args...)	\
+	pr_warn("zonefs (%s) WARN: " format, sb->s_id, ## args)
+
+#endif
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index f8c00045d537..57b627429f41 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -86,6 +86,7 @@ 
 #define NSFS_MAGIC		0x6e736673
 #define BPF_FS_MAGIC		0xcafe4a11
 #define AAFS_MAGIC		0x5a3c69f0
+#define ZONEFS_MAGIC		0x5a4f4653
 
 /* Since UDF 2.01 is ISO 13346 based... */
 #define UDF_SUPER_MAGIC		0x15013346