On Thu, May 11, 2017 at 06:50:23PM +0200, Jan Kara wrote:
> Currently several places in xfs_find_get_desired_pgoff() handle the case
> of a missing page. Make them all handled in one place after the loop has
> terminated.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/xfs/xfs_file.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index df51c025adfe..719923b99ba1 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1069,10 +1069,6 @@ xfs_find_get_desired_pgoff(
> break;
>
> ASSERT(type == HOLE_OFF);
> - if (lastoff == startoff || lastoff < endoff) {
> - found = true;
> - *offset = lastoff;
> - }
> break;
> }
>
> @@ -1080,11 +1076,8 @@ xfs_find_get_desired_pgoff(
> * At least we found one page. If the current offset is smaller
> * than the first page offset, a hole was found.
> */
> - if (type == HOLE_OFF && lastoff < page_offset(pvec.pages[0])) {
> - found = true;
> - *offset = lastoff;
> + if (type == HOLE_OFF && lastoff < page_offset(pvec.pages[0]))
> break;
> - }
>
> for (i = 0; i < nr_pages; i++) {
> struct page *page = pvec.pages[i];
FWIW, it looks like there's still a bug here that the first patch
doesn't address:
# xfs_io -fc "truncate 0" -c "falloc 0 32k" -c "pwrite 0 4k" -c "pwrite 32k 4k" -c "seek -h 0" /mnt/file
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0.0000 sec (126.008 MiB/sec and 32258.0645 ops/sec)
wrote 4096/4096 bytes at offset 32768
4 KiB, 1 ops; 0.0000 sec (102.796 MiB/sec and 26315.7895 ops/sec)
Whence Result
HOLE 36864
# xfs_io -c fsync -c "seek -h 0" /mnt/file
Whence Result
HOLE 4096
I initially thought this patch might fix it, but on further digging it
doesn't appear to. What looks like is going on here is that the
'page->index > end' check doesn't actually catch this case of the page
being just beyond the current unwritten extent (perhaps the check should
be 'page->index >= end'), so lastoff is set beyond end and we skip the
hole.
Brian
> @@ -1150,21 +1143,20 @@ xfs_find_get_desired_pgoff(
>
> /*
> * The number of returned pages less than our desired, search
> - * done. In this case, nothing was found for searching data,
> - * but we found a hole behind the last offset.
> + * done.
> */
> - if (nr_pages < want) {
> - if (type == HOLE_OFF) {
> - *offset = lastoff;
> - found = true;
> - }
> + if (nr_pages < want)
> break;
> - }
>
> index = pvec.pages[i - 1]->index + 1;
> pagevec_release(&pvec);
> } while (index <= end);
>
> + /* No page at lastoff and we are not done - we found a hole. */
> + if (type == HOLE_OFF && lastoff < endoff) {
> + *offset = lastoff;
> + found = true;
> + }
> out:
> pagevec_release(&pvec);
> return found;
> --
> 2.12.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
