fs core fixes (was Re: TFTP driver changes)

[Camilo Alejandro Arboleda]

Hi,

> My assumption is that this field is for use by a particular filesystem
implementation.
> Because the filesystem implementation may attach dynamically allocated
memory
> (or other resources) to 'node_access', both, the fs implementation (fsi)
and the
> framework provided by the filesystem core (coded in libc) must adhere to a
common
> policy regarding node allocation and cleanup.

>>From studying the source, it seems to me that the paradigm is that it's
the job
> of the 'evaluate and mount methods' (evalpath_h and evalformake_h
> as well as fsmount_me_h for initializing the mounted fs root node) to
initialize the
> node_access field if it's going to be used by other fsi routines.
However - the
> core must ensure the necessary calls of rtems_filesystem_freenode().
> It seems that rtems_filesystem_freenode()  _must_ be called on any
> nodes which yielded a successful evalpath_h(). Conversely, freenode()
> _must_not_ be called on any node for which evalpath() failed since the
> node can not be regarded as properly initialized.
>
> The appended patch fixes all deviations from this policy I found in libc.
>
> There are two more complex cases, however: open and unmount.
>
> 1) OPEN: a successful open allocates a rtems_libio_t structure and copies
>     the node which was obtained by a previous 'evalpath_h' into the libio
>     structure. IMHO, in this case (i.e. a _successful_ open), the node
_must_not_
>     be released because the node_access pointer copied into the libio
struct
>     could still be referenced in the future (by any fsi code operating on
the
>     open fd). Instead, rtems_filesystem_freenode() must be called by
'close'.

It is easy to manage from FS implementation. My node_access structure has a
'reference_counter', incremented in the following conditions:
- A successful eval_path_h or evalformake_h.
- A successful open.
It means that calling freenode_h after opening the file, *will not* cause
the node to be actually freed.

The reference counter is decreased in the following conditions:
- A call to freenode_h
- A successful file close.
It means that closing any file for last time, frees the node info.

I don't know how many people has implemented FS's, but I guess they took
care about the use of freenode in core filesystem support. If the core
filesystem is changed, the implementations should be too, to avoid memory
troubles.

> 2) There is another issue with MOUNT/UMOUNT: IMHO, the verification if the
>     target node is the fs root node is not quite useful for two reasons:
>       - a fs might not use node_access and set it to NULL for any node
>          which would identify any node as the fs root node.
>       - a fs using node_access might attach equivalent but not identical
>          objects to node access (i.e. copies of the same string) to
node_access
>          which would cause the verification to fail although the target
>          node _is_ indeed the fs root node.
>     Possible solutions could be:
>     a) (umount only) comparing all the other
>     fields (but node_access) leading to slightly different semantics for
umount
>     (unmount a fs by referencing any file on the fs).
>     b) adding a flag to filesystem_location_info_t identifying
>     a fs root node.
>
>     Thinking this over a little bit, I guess the current behavior might be
>     acceptible. However, fs implementors should set the node_access
>     field of the fs root to some special value thereby tagging it
>     as the root node; on any other node, the fsi must set node_access to
>     some other value - a not very clean solution.
>
>     Other ideas??

I think current behavior is ok. The only care you must take is not
dupplicating node info. If you open a file twice, boot iop->node_access
should point to the same node info. It solves the problem you point with
root node, and allows you know how many times the file has been opened.

Camilo

> Note that the attached patch has not been thoroughly tested and needs
> inspection by a fs expert.
>
> Regards, (sorry for the long message)
>
> -- Till.
>


----------------------------------------------------------------------------
----