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

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

[Hugh Dickins]

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?

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>
---
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;
 			}
 		}
 

--
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>

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

[Michal Hocko]

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>

> ---
> 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>

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

[Johannes Weiner]

On Wed, Aug 19, 2015 at 07:36:31PM -0700, 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?
> 
> 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: Johannes Weiner <hannes@cmpxchg.org>

--
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>