linux-ext4 - Re: [PATCHv5 6/6] Documentation: Correct the description of FIEMAP_EXTENT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Feb 2020 07:36:50 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Ritesh Harjani <riteshh@...ux.ibm.com>
Cc:     linux-ext4@...r.kernel.org, jack@...e.cz, tytso@....edu,
        adilger.kernel@...ger.ca, linux-fsdevel@...r.kernel.org,
        darrick.wong@...cle.com, hch@...radead.org, cmaiolino@...hat.com,
        david@...morbit.com
Subject: Re: [PATCHv5 6/6] Documentation: Correct the description of
 FIEMAP_EXTENT_LAST

On Fri, Feb 28, 2020 at 02:56:59PM +0530, Ritesh Harjani wrote:
> Currently FIEMAP_EXTENT_LAST is not working consistently across
> different filesystem's fiemap implementations. So add more information
> about how else this flag could set in other implementation.
> 
> Also in general, user should not completely rely on this flag as
> such since it could return false value for e.g.
> when there is a delalloc extent which might get converted during
> writeback, immediately after the fiemap calls return.
> 
> Signed-off-by: Ritesh Harjani <riteshh@...ux.ibm.com>
> ---
>  Documentation/filesystems/fiemap.txt | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/filesystems/fiemap.txt b/Documentation/filesystems/fiemap.txt
> index f6d9c99103a4..fedfa9b9dde5 100644
> --- a/Documentation/filesystems/fiemap.txt
> +++ b/Documentation/filesystems/fiemap.txt
> @@ -71,8 +71,7 @@ allocated is less than would be required to map the requested range,
>  the maximum number of extents that can be mapped in the fm_extent[]
>  array will be returned and fm_mapped_extents will be equal to
>  fm_extent_count. In that case, the last extent in the array will not
> -complete the requested range and will not have the FIEMAP_EXTENT_LAST
> -flag set (see the next section on extent flags).
> +complete the requested range.

This sentence still seems like it should be true.  If the filesystem knows
there are more extents to come, it will definitely not set the LAST flag.

> @@ -96,7 +95,7 @@ block size of the file system.  With the exception of extents flagged as
>  FIEMAP_EXTENT_MERGED, adjacent extents will not be merged.
>  
>  The fe_flags field contains flags which describe the extent returned.
> -A special flag, FIEMAP_EXTENT_LAST is always set on the last extent in
> +A special flag, FIEMAP_EXTENT_LAST *may be* set on the last extent in
>  the file so that the process making fiemap calls can determine when no
>  more extents are available, without having to call the ioctl again.

I'm not sure I'd highlight 'may be' here.

> @@ -115,8 +114,9 @@ data. Note that the opposite is not true - it would be valid for
>  FIEMAP_EXTENT_NOT_ALIGNED to appear alone.
>  
>  * FIEMAP_EXTENT_LAST
> -This is the last extent in the file. A mapping attempt past this
> -extent will return nothing.
> +This is generally the last extent in the file. A mapping attempt past this
> +extent may return nothing. In some implementations this flag is also set on
> +the last dataset queried by the user (via fiemap->fm_length).

The word 'dataset' is used nowhere else in this document.  How about

"Some filesystems set this flag to indicate this extent is the last one in
the range queried by the user"