Re: [PATCH V3] fs: New zonefs file system

[Damien Le Moal <Damien.LeMoal@wdc.com>]

Hannes,

On 2019/08/23 19:12, Hannes Reinecke wrote:
> On 8/21/19 9:03 AM, Damien Le Moal wrote:
[...]>> @@ -261,6 +262,7 @@ source "fs/romfs/Kconfig"
>>  source "fs/pstore/Kconfig"
>>  source "fs/sysv/Kconfig"
>>  source "fs/ufs/Kconfig"
>> +source "fs/ufs/Kconfig"
>>  
>>  endif # MISC_FILESYSTEMS
>>  
> Hmm?
> Duplicate line?

Ooops... Something is wrong with my rebase since there is no need to touch this
file. Will check.

> 
>> diff --git a/fs/Makefile b/fs/Makefile
>> index d60089fd689b..7d3c90e1ad79 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..5521c21fd34b
>> --- /dev/null
>> +++ b/fs/zonefs/super.c
> [ .. ]
> 
> That whole thing looks good to me (with my limited fs skills :-),
> however, some things I'd like to have clarified:
> 
> - zone state handling:
> While you do have some handling for offline zones, I'm missing a
> handling during normal I/O. Surely a zone can go offline via other means
> (like the admin calling nasty user-space programs), which then would
> result in an I/O error in the filesystem.
> Shouldn't we handle this case when doing error handling?

Yes, we could. But whatever we do, the only solution is to return an IO error to
the user for any IO request directed at an offline zone. Without the special
offline zone handling code, the drive fails all read/write request to the
offline zone, resulting in the same behavior. So I do not see this as critical
and could be later improvements.

But I can see that for a file representing an offline zone, changing the file
size to 0 and/or removing all access rights to the file would be a good idea.

> IE shouldn't we look at the zone state when doing a REPORT ZONES, and
> update it if required?

That is what the error handling path should do, yes.

> Similarly: How do we present zones which are not accessible? Will they
> still show up in the directory? I think they should, but we should be
> returning an error to userspace like EPERM or somesuch.

Right now, they are not added to the directory as files, so not visible.
However, that is a mistake since if zones go offline between 2 mounts, the file
names will shift, which is of course not acceptable. I will fix that and expose
even offline zones files, but make them not accessible.

> - zone sizes:
> From what I've seen sequential zones can be appended to, ie they'll
> start off at 0 and will increase in size. Conventional zones, OTOH,
> apparently always have a fixed size. Is that correct?

Yes, correct. Conventional zone files have a fixed size (the zone size or the
sum of all contiguous conventional zones for aggregated mount) and cannot be
truncated. They can be overwritten randomly.

Best regards.


-- 
Damien Le Moal
Western Digital Research