| Message ID | 5a00e8d4283d6849e0b8f408c8365b31fbc1d153.1582880246.git.riteshh@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ext4: bmap & fiemap conversion to use iomap | expand |
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@linux.ibm.com> Looks reasonable, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > 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. > > Each extent is described by a single fiemap_extent structure as > returned in fm_extents. > @@ -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. > > @@ -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). > > * FIEMAP_EXTENT_UNKNOWN > The location of this extent is currently unknown. This may indicate > -- > 2.21.0 >
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@linux.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"
On 2/28/20 9:06 PM, Matthew Wilcox wrote: > 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@linux.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. > sure. >> @@ -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. Sure. > >> @@ -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" Sure. Thanks for the review. Will make the suggested changes and send a v6. -ritesh
On Mon, Mar 02, 2020 at 01:40:06PM +0530, Ritesh Harjani wrote: > > Thanks for the review. > Will make the suggested changes and send a v6. I didn't see a v6, so I revised this patch to read: commit 499800830ae5d44ae29f69c98ab9893f0425cb51 Author: Ritesh Harjani <riteshh@linux.ibm.com> Date: Fri Feb 28 14:56:59 2020 +0530 Documentation: correct the description of FIEMAP_EXTENT_LAST 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. Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> Link: https://lore.kernel.org/r/5a00e8d4283d6849e0b8f408c8365b31fbc1d153.1582880246.git.riteshh@linux.ibm.com Signed-off-by: Theodore Ts'o <tytso@mit.edu> diff --git a/Documentation/filesystems/fiemap.txt b/Documentation/filesystems/fiemap.txt index f6d9c99103a4..ac87e6fda842 100644 --- a/Documentation/filesystems/fiemap.txt +++ b/Documentation/filesystems/fiemap.txt @@ -115,8 +115,10 @@ 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. Some implementations set this flag to +indicate this extent is the last one in the range queried by the user +(via fiemap->fm_length). * FIEMAP_EXTENT_UNKNOWN The location of this extent is currently unknown. This may indicate - Ted
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. Each extent is described by a single fiemap_extent structure as returned in fm_extents. @@ -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. @@ -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). * FIEMAP_EXTENT_UNKNOWN The location of this extent is currently unknown. This may indicate
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@linux.ibm.com> --- Documentation/filesystems/fiemap.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)