Thread (3 messages) 3 messages, 3 authors, 2015-08-21

Re: [PATCH] mm, vmscan: unlock page while waiting on writeback

From: Michal Hocko <hidden>
Date: 2015-08-20 08:09:36

On Wed 19-08-15 19:36:31, Hugh Dickins wrote:
This is merely a politeness: I've not found that shrink_page_list() leads
to deadlock with the page it holds locked across wait_on_page_writeback();
but nevertheless, why hold others off by keeping the page locked there?
OK, this makes sense to me. It is safer to wait without page locked.
Maybe some fs will want to lock the page before clearing the page
writeback in the future and we would be broken in a very subtle way.
And while we're at it: remove the mistaken "not " from the commentary
on this Case 3 (and a distracting blank line from Case 2, if I may).

Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
quoted hunk ↗ jump to hunk
---
I remembered this old patch when we were discussing the more important
ecf5fc6e9654 "mm, vmscan: Do not wait for page writeback for GFP_NOFS
allocations", and now retested it against mmotm.

 mm/vmscan.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
--- mmotm/mm/vmscan.c	2015-08-17 18:46:26.601521575 -0700
+++ linux/mm/vmscan.c	2015-08-17 18:53:41.335108240 -0700
@@ -991,7 +991,7 @@ static unsigned long shrink_page_list(st
 		 *    __GFP_IO|__GFP_FS for this reason); but more thought
 		 *    would probably show more reasons.
 		 *
-		 * 3) Legacy memcg encounters a page that is not already marked
+		 * 3) Legacy memcg encounters a page that is already marked
 		 *    PageReclaim. memcg does not have any dirty pages
 		 *    throttling so we could easily OOM just because too many
 		 *    pages are in writeback and there is nothing else to
@@ -1021,12 +1021,15 @@ static unsigned long shrink_page_list(st
 				 */
 				SetPageReclaim(page);
 				nr_writeback++;
-
 				goto keep_locked;
 
 			/* Case 3 above */
 			} else {
+				unlock_page(page);
 				wait_on_page_writeback(page);
+				/* then go back and try same page again */
+				list_add_tail(&page->lru, page_list);
+				continue;
 			}
 		}
 
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help