On 05/02/23 23:56, Ackerley Tng wrote:
> When fallocate() is called twice on the same offset in the file, the
> second fallocate() should succeed.
>
> page_cache_next_miss() always advances index before returning, so even
> on a page cache hit, the check would set present to false.
Thank you Ackerley for finding this!
When I read the description of page_cache_next_miss(), I assumed
present = page_cache_next_miss(mapping, index, 1) != index;
would tell us if there was a page at index in the cache.
However, when looking closer at the code it does not check for a page
at index, but rather starts looking at index+1. Perhaps that is why
it is named next?
Matthew, I think the use of the above statement was your suggestion.
And you know the xarray code better than anyone. I just want to make
sure page_cache_next_miss is operating as designed/expected. If so,
then the changes suggested here make sense.
In addition, the same code is in hugetlbfs_pagecache_present and will
have this same issue.
--
Mike Kravetz
>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> ---
> fs/hugetlbfs/inode.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index ecfdfb2529a3..f640cff1bbce 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -821,7 +821,6 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
> */
> struct folio *folio;
> unsigned long addr;
> - bool present;
>
> cond_resched();
>
> @@ -845,10 +844,7 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
> mutex_lock(&hugetlb_fault_mutex_table[hash]);
>
> /* See if already present in mapping to avoid alloc/free */
> - rcu_read_lock();
> - present = page_cache_next_miss(mapping, index, 1) != index;
> - rcu_read_unlock();
> - if (present) {
> + if (filemap_has_folio(mapping, index)) {
> mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> hugetlb_drop_vma_policy(&pseudo_vma);
> continue;
> --
> 2.40.1.495.gc816e09b53d-goog
>
