Re: [PATCH 08/12] docs: path-lookup: update i_op->put_link and cookie description

[NeilBrown <neilb@suse.de>]

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

On Tue, Jan 26 2021, Fox Chen wrote:

> No inode->put_link operation anymore. We use delayed_call to
> deal with link destruction. Cookie has been replaced with
> struct delayed_call.
>
> Related commit: fceef393a538134f03b778c5d2519e670269342f
>
> Signed-off-by: Fox Chen <foxhlchen@gmail.com>
> ---
>  Documentation/filesystems/path-lookup.rst | 31 +++++++----------------
>  1 file changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/filesystems/path-lookup.rst b/Documentation/filesystems/path-lookup.rst
> index 0a362849b26f..8572300b5405 100644
> --- a/Documentation/filesystems/path-lookup.rst
> +++ b/Documentation/filesystems/path-lookup.rst
> @@ -1068,34 +1068,21 @@ method. This is called both in RCU-walk and REF-walk. In RCU-walk the
>  RCU-walk.  Much like the ``i_op->permission()`` method we
>  looked at previously, ``->get_link()`` would need to be careful that
>  all the data structures it references are safe to be accessed while
> -holding no counted reference, only the RCU lock.  Though getting a
> -reference with ``->follow_link()`` is not yet done in RCU-walk mode, the
> -code is ready to release the reference when that does happen.
> -
> -This need to drop the reference to a symlink adds significant
> -complexity.  It requires a reference to the inode so that the
> -``i_op->put_link()`` inode operation can be called.  In REF-walk, that
> -reference is kept implicitly through a reference to the dentry, so
> -keeping the ``struct path`` of the symlink is easiest.  For RCU-walk,
> -the pointer to the inode is kept separately.  To allow switching from
> -RCU-walk back to REF-walk in the middle of processing nested symlinks
> -we also need the seq number for the dentry so we can confirm that
> -switching back was safe.
> -
> -Finally, when providing a reference to a symlink, the filesystem also
> -provides an opaque "cookie" that must be passed to ``->put_link()`` so that it
> -knows what to free.  This might be the allocated memory area, or a
> -pointer to the ``struct page`` in the page cache, or something else
> -completely.  Only the filesystem knows what it is.
> +holding no counted reference, only the RCU lock.
> +
> +Finally, a callback ``struct delayed_called`` will be passed to get_link,
> +file systems can set their own put_link function and argument through
> +``set_delayed_call``. Latter on, when vfs wants to put link, it will call 

"Later", not "Latter".

Also, I'm not sure that the "Finally" at the start of the sentence makes
sense any more.  I think it was meant to introduce the final part of the
"significant complexity", but now that significant complexity is gone.
At least, I assume it is gone.  I haven't checked to code to see if
maybe it has just been moved.

NeilBrown


> +``do_delayed_call`` to invoke that callback function with the argument.
>  
>  In order for the reference to each symlink to be dropped when the walk completes,
>  whether in RCU-walk or REF-walk, the symlink stack needs to contain,
>  along with the path remnants:
>  
> -- the ``struct path`` to provide a reference to the inode in REF-walk
> -- the ``struct inode *`` to provide a reference to the inode in RCU-walk
> +- the ``struct path`` to provide a reference to the previous path
> +- the ``const char *`` to provide a reference to the to previous name
>  - the ``seq`` to allow the path to be safely switched from RCU-walk to REF-walk
> -- the ``cookie`` that tells ``->put_path()`` what to put.
> +- the ``struct delayed_call`` for later invocation.
>  
>  This means that each entry in the symlink stack needs to hold five
>  pointers and an integer instead of just one pointer (the path
> -- 
> 2.30.0

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