Reconsidering exportable UBIFS

Reconsidering exportable UBIFS

[Richard Weinberger]

Hi!

Currently UBIFS is not exportable.
I'm not sure whether it is completely impossible or if I just miss a detail.
So I've some questions.

Documentation/filesystems/nfs/Exporting states that the only required function
is fh_to_dentry().
This function should on UBIFS be implementable using generic_fh_to_dentry().
While UBIFS reuses in theory inode numbers we can ignore i_generation as
the flash chip is long dead before we start reusing inodes. Same as for JFFS2.
But the same document states also that fh_to_parent() and get_parent() are optional
but strongly recommended.
What does this mean? Will NFS work but puppies die and turn into zombies?

Implementing get_parent() is a little unpleasant.
UBIFS's on-flash layout does not support querying the parent.
We could change UBIFS's struct ubifs_ino_node, but I'd change the
on-flash layout only as last resort.

The biggest problem I see is that UBIFS does not really support telldir()
and seekdir().
Directory offsets in UBIFS are plain hash values, so telldir()/seekdir() won't
correctly work if UBIFS faces hash collisions.
Currently UBIFS implements a hack which stores the UBIFS dent object into
file->private_data such that consecutive readdir()s are guaranteed to work.
A comment on UBIFS's readdir states:
 * This means that UBIFS cannot support NFS which requires full
 * 'seekdir()'/'telldir()' support.

Is this still true? Maybe we can have NFS even if it is not perfect in
terms of performance.

Artem, did I miss another show stopper? :-)

Thanks,
//richard

Re: Reconsidering exportable UBIFS

[Artem Bityutskiy]

On Tue, 2016-04-05 at 00:17 +0200, Richard Weinberger wrote:
> The biggest problem I see is that UBIFS does not really support
> telldir()
> and seekdir().
> Directory offsets in UBIFS are plain hash values, so
> telldir()/seekdir() won't
> correctly work if UBIFS faces hash collisions.
> Currently UBIFS implements a hack which stores the UBIFS dent object
> into
> file->private_data such that consecutive readdir()s are guaranteed to
> work.
> A comment on UBIFS's readdir states:
>  * This means that UBIFS cannot support NFS which requires full
>  * 'seekdir()'/'telldir()' support.

Richard, I do not remember much about NFS support. I can only say that
I never really put efforts into this, unfortunately.
Here is an old discussion which could give you some more hints:

http://marc.info/?l=linux-nfsv4&m=126480082613293&w=2

Artem.

Re: Reconsidering exportable UBIFS

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]


(added linux-nfs@vger.kernel.org)

On Tue, Apr 05 2016, Richard Weinberger wrote:

> Hi!
>
> Currently UBIFS is not exportable.
> I'm not sure whether it is completely impossible or if I just miss a detail.
> So I've some questions.
>
> Documentation/filesystems/nfs/Exporting states that the only required function
> is fh_to_dentry().
> This function should on UBIFS be implementable using generic_fh_to_dentry().
> While UBIFS reuses in theory inode numbers we can ignore i_generation as
> the flash chip is long dead before we start reusing inodes. Same as for JFFS2.
> But the same document states also that fh_to_parent() and get_parent() are optional
> but strongly recommended.
> What does this mean? Will NFS work but puppies die and turn into
> zombies?

Not puppies, just kittens.

If you don't provide these functions, then exporting with
"subtree_check" won't work.  That is no great loss except that people
might find the failure confusing.


>
> Implementing get_parent() is a little unpleasant.
> UBIFS's on-flash layout does not support querying the parent.
> We could change UBIFS's struct ubifs_ino_node, but I'd change the
> on-flash layout only as last resort.
>
> The biggest problem I see is that UBIFS does not really support telldir()
> and seekdir().
> Directory offsets in UBIFS are plain hash values, so telldir()/seekdir() won't
> correctly work if UBIFS faces hash collisions.
> Currently UBIFS implements a hack which stores the UBIFS dent object into
> file->private_data such that consecutive readdir()s are guaranteed to work.
> A comment on UBIFS's readdir states:
>  * This means that UBIFS cannot support NFS which requires full
>  * 'seekdir()'/'telldir()' support.
>
> Is this still true? Maybe we can have NFS even if it is not perfect in
> terms of performance.

How big are your hashes?
ext3 messed up their readdir/telldir design too so they don't have
guaranteed unique keys.
When using 32bit hashes you can definitely get problems with
collisions.  I have not heard of problems with 64bit hashes.

I may have the details slightly wrong, but as I recall non-uniqueness of
cookies only causes a problem when the last cookie returned in a READDIR
reply matches the first cookie returned in reply to the next readdir.
So non-uniqueness is only a problem when it aligns badly.

NeilBrown


>
> Artem, did I miss another show stopper? :-)
>
> Thanks,
> //richard
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: Reconsidering exportable UBIFS

[Al Viro]

On Mon, May 09, 2016 at 08:18:22AM +1000, NeilBrown wrote:

> Not puppies, just kittens.
> 
> If you don't provide these functions, then exporting with
> "subtree_check" won't work.  That is no great loss except that people
> might find the failure confusing.

	OK, a client sends you a RENAME.  With fhandles of both parents +
old and new names in those.  Your task, should you choose to accept it, is
to figure out whether we should fail with nfserr_inval due to an attempt
to make a directory its own descendent.  Without being able to locate all
ancestors of a directory.

	You are fond of complaining about the checks that could've been left
to server not getting skipped on the client.  Now you want to skip them on
the server side as well?  Can't have it both ways...

	Seriously, it really doesn't work.  You can't do directory
modifications without having found the chain of ancestors.  No ->get_parent()
is OK _only_ for something like tmpfs, where we have the full chains of
ancestors towards root all the time.  For UBIFS it's obviously not true.
Not unless you suck the entire directory tree in memory at the mount time.

Re: Reconsidering exportable UBIFS

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On Mon, May 09 2016, Al Viro wrote:

> On Mon, May 09, 2016 at 08:18:22AM +1000, NeilBrown wrote:
>
>> Not puppies, just kittens.
>> 
>> If you don't provide these functions, then exporting with
>> "subtree_check" won't work.  That is no great loss except that people
>> might find the failure confusing.
>
> 	OK, a client sends you a RENAME.  With fhandles of both parents +
> old and new names in those.  Your task, should you choose to accept it, is
> to figure out whether we should fail with nfserr_inval due to an attempt
> to make a directory its own descendent.  Without being able to locate all
> ancestors of a directory.

You are right, sorry.  I was thinking that get_parent() was for finding
the parent of a non-directory, but it is for directories.  It does the
equivalent of lookup("..").  So if you have a ".." link or something
like it, it should be easy.  If you don't, it won't be easy at all.

Thanks,
NeilBrown


>
> 	You are fond of complaining about the checks that could've been left
> to server not getting skipped on the client.  Now you want to skip them on
> the server side as well?  Can't have it both ways...
>
> 	Seriously, it really doesn't work.  You can't do directory
> modifications without having found the chain of ancestors.  No ->get_parent()
> is OK _only_ for something like tmpfs, where we have the full chains of
> ancestors towards root all the time.  For UBIFS it's obviously not true.
> Not unless you suck the entire directory tree in memory at the mount time.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: Reconsidering exportable UBIFS

[Richard Weinberger]

Am 09.05.2016 um 07:03 schrieb NeilBrown:
> On Mon, May 09 2016, Al Viro wrote:
> 
>> On Mon, May 09, 2016 at 08:18:22AM +1000, NeilBrown wrote:
>>
>>> Not puppies, just kittens.
>>>
>>> If you don't provide these functions, then exporting with
>>> "subtree_check" won't work.  That is no great loss except that people
>>> might find the failure confusing.
>>
>> 	OK, a client sends you a RENAME.  With fhandles of both parents +
>> old and new names in those.  Your task, should you choose to accept it, is
>> to figure out whether we should fail with nfserr_inval due to an attempt
>> to make a directory its own descendent.  Without being able to locate all
>> ancestors of a directory.
> 
> You are right, sorry.  I was thinking that get_parent() was for finding
> the parent of a non-directory, but it is for directories.  It does the
> equivalent of lookup("..").  So if you have a ".." link or something
> like it, it should be easy.  If you don't, it won't be easy at all.

Thanks for the detailed explanation.
UBIFS does not have ".." links in the on-flash layout.
So, we'd have to change this.

Thanks,
//richard

Re: Reconsidering exportable UBIFS

[Richard Weinberger]

Am 09.05.2016 um 00:18 schrieb NeilBrown:
>> The biggest problem I see is that UBIFS does not really support telldir()
>> and seekdir().
>> Directory offsets in UBIFS are plain hash values, so telldir()/seekdir() won't
>> correctly work if UBIFS faces hash collisions.
>> Currently UBIFS implements a hack which stores the UBIFS dent object into
>> file->private_data such that consecutive readdir()s are guaranteed to work.
>> A comment on UBIFS's readdir states:
>>  * This means that UBIFS cannot support NFS which requires full
>>  * 'seekdir()'/'telldir()' support.
>>
>> Is this still true? Maybe we can have NFS even if it is not perfect in
>> terms of performance.
> 
> How big are your hashes?
> ext3 messed up their readdir/telldir design too so they don't have
> guaranteed unique keys.
> When using 32bit hashes you can definitely get problems with
> collisions.  I have not heard of problems with 64bit hashes.
> 
> I may have the details slightly wrong, but as I recall non-uniqueness of
> cookies only causes a problem when the last cookie returned in a READDIR
> reply matches the first cookie returned in reply to the next readdir.
> So non-uniqueness is only a problem when it aligns badly.

UBIFS is using 32bit hashes, r5 hash from reiserfs. ;-\

Thanks,
//richard