Thread (20 messages) 20 messages, 3 authors, 2021-07-13

Re: [PATCH 03/14] mm: Protect operations adding pages to page cache with invalidate_lock

From: Jan Kara <jack@suse.cz>
Date: 2021-07-13 12:35:52
Also in: ceph-devel, linux-cifs, linux-f2fs-devel, linux-fsdevel, linux-mm, linux-xfs

On Tue 13-07-21 07:25:05, Christoph Hellwig wrote:
Still looks good.  That being said the additional conditional locking in
filemap_fault makes it fall over the readbility cliff for me.  Something
like this on top of your series would help:
Yeah, that's better. Applied, thanks. 

								Honza
quoted hunk ↗ jump to hunk
diff --git a/mm/filemap.c b/mm/filemap.c
index fd3f94d36c49..0fad08331cf4 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3040,21 +3040,23 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
 	 * Do we have something in the page cache already?
 	 */
 	page = find_get_page(mapping, offset);
-	if (likely(page) && !(vmf->flags & FAULT_FLAG_TRIED)) {
+	if (likely(page)) {
 		/*
-		 * We found the page, so try async readahead before
-		 * waiting for the lock.
+		 * We found the page, so try async readahead before waiting for
+		 * the lock.
 		 */
-		fpin = do_async_mmap_readahead(vmf, page);
-	} else if (!page) {
+		if (!(vmf->flags & FAULT_FLAG_TRIED))
+			fpin = do_async_mmap_readahead(vmf, page);
+		if (unlikely(!PageUptodate(page))) {
+			filemap_invalidate_lock_shared(mapping);
+			mapping_locked = true;
+		}
+	} else {
 		/* No page in the page cache at all */
 		count_vm_event(PGMAJFAULT);
 		count_memcg_event_mm(vmf->vma->vm_mm, PGMAJFAULT);
 		ret = VM_FAULT_MAJOR;
 		fpin = do_sync_mmap_readahead(vmf);
-	}
-
-	if (!page) {
 retry_find:
 		/*
 		 * See comment in filemap_create_page() why we need
@@ -3073,9 +3075,6 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
 			filemap_invalidate_unlock_shared(mapping);
 			return VM_FAULT_OOM;
 		}
-	} else if (unlikely(!PageUptodate(page))) {
-		filemap_invalidate_lock_shared(mapping);
-		mapping_locked = true;
 	}
 
 	if (!lock_page_maybe_drop_mmap(vmf, page, &fpin))
-- 
Jan Kara [off-list ref]
SUSE Labs, CR
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help