| Message ID | efa86091-6a2c-4064-8f55-9b44e1313015@moroto.mountain (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/hugetlb: fix hugetlbfs_pagecache_present() | expand |
On 6/22/23 11:26 PM, Dan Carpenter wrote: > The filemap_get_folio() function doesn't returns NULL, it returns error > pointers. So the "return folio != NULL;" statement means > hugetlbfs_pagecache_present() always returns true. > > Fixes: 267d6792f43b ("hugetlb: revert use of page_cache_next_miss()") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > mm/hugetlb.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index cb9077b96b43..bce28cca73a1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -5731,9 +5731,10 @@ static bool hugetlbfs_pagecache_present(struct hstate *h, > struct folio *folio; > > folio = filemap_get_folio(mapping, idx); > - if (!IS_ERR(folio)) > - folio_put(folio); > - return folio != NULL; > + if (IS_ERR(folio)) > + return false; > + folio_put(folio); > + return true; > } > > int hugetlb_add_to_page_cache(struct folio *folio, struct address_space *mapping, Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
On 23.06.23 08:26, Dan Carpenter wrote: > The filemap_get_folio() function doesn't returns NULL, it returns error > pointers. So the "return folio != NULL;" statement means > hugetlbfs_pagecache_present() always returns true. > > Fixes: 267d6792f43b ("hugetlb: revert use of page_cache_next_miss()") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > mm/hugetlb.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index cb9077b96b43..bce28cca73a1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -5731,9 +5731,10 @@ static bool hugetlbfs_pagecache_present(struct hstate *h, > struct folio *folio; > > folio = filemap_get_folio(mapping, idx); > - if (!IS_ERR(folio)) > - folio_put(folio); > - return folio != NULL; > + if (IS_ERR(folio)) > + return false; > + folio_put(folio); > + return true; > } > > int hugetlb_add_to_page_cache(struct folio *folio, struct address_space *mapping, Reviewed-by: David Hildenbrand <david@redhat.com>
On 06/23/23 09:26, Dan Carpenter wrote: > The filemap_get_folio() function doesn't returns NULL, it returns error > pointers. So the "return folio != NULL;" statement means > hugetlbfs_pagecache_present() always returns true. > > Fixes: 267d6792f43b ("hugetlb: revert use of page_cache_next_miss()") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > mm/hugetlb.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index cb9077b96b43..bce28cca73a1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -5731,9 +5731,10 @@ static bool hugetlbfs_pagecache_present(struct hstate *h, > struct folio *folio; > > folio = filemap_get_folio(mapping, idx); > - if (!IS_ERR(folio)) > - folio_put(folio); > - return folio != NULL; > + if (IS_ERR(folio)) > + return false; > + folio_put(folio); > + return true; > } Thank you Dan! I noted the changed behavior of filemap_get_folio in the commit message and still missed that comparison to NULL. :(
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index cb9077b96b43..bce28cca73a1 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5731,9 +5731,10 @@ static bool hugetlbfs_pagecache_present(struct hstate *h, struct folio *folio; folio = filemap_get_folio(mapping, idx); - if (!IS_ERR(folio)) - folio_put(folio); - return folio != NULL; + if (IS_ERR(folio)) + return false; + folio_put(folio); + return true; } int hugetlb_add_to_page_cache(struct folio *folio, struct address_space *mapping,
The filemap_get_folio() function doesn't returns NULL, it returns error pointers. So the "return folio != NULL;" statement means hugetlbfs_pagecache_present() always returns true. Fixes: 267d6792f43b ("hugetlb: revert use of page_cache_next_miss()") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- mm/hugetlb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)