Hello Christoph Hellwig,
The patch 480c454ff64b: "mm: return an ERR_PTR from
__filemap_get_folio" from Mar 7, 2023, leads to the following Smatch
static checker warning:
mm/filemap.c:3381 filemap_fault()
warn: 'folio' is an error pointer or valid
mm/filemap.c
3262 folio = filemap_get_folio(mapping, index);
3263 if (likely(!IS_ERR(folio))) {
3264 /*
3265 * We found the page, so try async readahead before waiting for
3266 * the lock.
3267 */
3268 if (!(vmf->flags & FAULT_FLAG_TRIED))
3269 fpin = do_async_mmap_readahead(vmf, folio);
3270 if (unlikely(!folio_test_uptodate(folio))) {
3271 filemap_invalidate_lock_shared(mapping);
3272 mapping_locked = true;
3273 }
3274 } else {
3275 /* No page in the page cache at all */
3276 count_vm_event(PGMAJFAULT);
3277 count_memcg_event_mm(vmf->vma->vm_mm, PGMAJFAULT);
3278 ret = VM_FAULT_MAJOR;
3279 fpin = do_sync_mmap_readahead(vmf);
3280 retry_find:
3281 /*
3282 * See comment in filemap_create_folio() why we need
3283 * invalidate_lock
3284 */
3285 if (!mapping_locked) {
3286 filemap_invalidate_lock_shared(mapping);
3287 mapping_locked = true;
3288 }
3289 folio = __filemap_get_folio(mapping, index,
3290 FGP_CREAT|FGP_FOR_MMAP,
3291 vmf->gfp_mask);
This function was changed to return error pointers instead of NULL.
3292 if (IS_ERR(folio)) {
3293 if (fpin)
3294 goto out_retry;
3295 filemap_invalidate_unlock_shared(mapping);
3296 return VM_FAULT_OOM;
3297 }
3298 }
3299
3300 if (!lock_folio_maybe_drop_mmap(vmf, folio, &fpin))
3301 goto out_retry;
3302
3303 /* Did it get truncated? */
3304 if (unlikely(folio->mapping != mapping)) {
3305 folio_unlock(folio);
3306 folio_put(folio);
3307 goto retry_find;
3308 }
3309 VM_BUG_ON_FOLIO(!folio_contains(folio, index), folio);
3310
3311 /*
3312 * We have a locked page in the page cache, now we need to check
3313 * that it's up-to-date. If not, it is going to be due to an error.
3314 */
3315 if (unlikely(!folio_test_uptodate(folio))) {
3316 /*
3317 * The page was in cache and uptodate and now it is not.
3318 * Strange but possible since we didn't hold the page lock all
3319 * the time. Let's drop everything get the invalidate lock and
3320 * try again.
3321 */
3322 if (!mapping_locked) {
3323 folio_unlock(folio);
3324 folio_put(folio);
3325 goto retry_find;
3326 }
3327 goto page_not_uptodate;
3328 }
3329
3330 /*
3331 * We've made it this far and we had to drop our mmap_lock, now is the
3332 * time to return to the upper layer and have it re-find the vma and
3333 * redo the fault.
3334 */
3335 if (fpin) {
3336 folio_unlock(folio);
3337 goto out_retry;
3338 }
3339 if (mapping_locked)
3340 filemap_invalidate_unlock_shared(mapping);
3341
3342 /*
3343 * Found the page and have a reference on it.
3344 * We must recheck i_size under page lock.
3345 */
3346 max_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
3347 if (unlikely(index >= max_idx)) {
3348 folio_unlock(folio);
3349 folio_put(folio);
3350 return VM_FAULT_SIGBUS;
3351 }
3352
3353 vmf->page = folio_file_page(folio, index);
3354 return ret | VM_FAULT_LOCKED;
3355
3356 page_not_uptodate:
3357 /*
3358 * Umm, take care of errors if the page isn't up-to-date.
3359 * Try to re-read it _once_. We do this synchronously,
3360 * because there really aren't any performance issues here
3361 * and we need to check for errors.
3362 */
3363 fpin = maybe_unlock_mmap_for_io(vmf, fpin);
3364 error = filemap_read_folio(file, mapping->a_ops->read_folio, folio);
3365 if (fpin)
3366 goto out_retry;
3367 folio_put(folio);
3368
3369 if (!error || error == AOP_TRUNCATED_PAGE)
3370 goto retry_find;
3371 filemap_invalidate_unlock_shared(mapping);
3372
3373 return VM_FAULT_SIGBUS;
3374
3375 out_retry:
3376 /*
3377 * We dropped the mmap_lock, we need to return to the fault handler to
3378 * re-find the vma and come back and find our hopefully still populated
3379 * page.
3380 */
3381 if (folio)
^^^^^
Should this be !IS_ERR() now?
3382 folio_put(folio);
3383 if (mapping_locked)
3384 filemap_invalidate_unlock_shared(mapping);
3385 if (fpin)
3386 fput(fpin);
3387 return ret | VM_FAULT_RETRY;
3388 }
regards,
dan carpenter
