From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 14 Dec 2018 09:49:48 +0100 From: Michal Hocko To: Johannes Weiner Cc: Andrew Morton , "Kirill A. Shutemov" , Liu Bo , Jan Kara , Dave Chinner , Theodore Ts'o , Vladimir Davydov , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, LKML , Shakeel Butt , Stable tree Subject: Re: [PATCH v3] mm, memcg: fix reclaim deadlock with writeback Message-ID: <20181214084948.GA5624@dhcp22.suse.cz> References: <20181212155055.1269-1-mhocko@kernel.org> <20181213092221.27270-1-mhocko@kernel.org> <20181213220400.GA9829@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181213220400.GA9829@cmpxchg.org> Sender: owner-linux-mm@kvack.org List-ID: On Thu 13-12-18 17:04:00, Johannes Weiner wrote: [...] > Acked-by: Johannes Weiner Thanks! > Just one nit: > > > @@ -2993,6 +2993,17 @@ static vm_fault_t __do_fault(struct vm_fault *vmf) > > struct vm_area_struct *vma = vmf->vma; > > vm_fault_t ret; > > > > + /* > > + * Preallocate pte before we take page_lock because this might lead to > > + * deadlocks for memcg reclaim which waits for pages under writeback. > > + */ > > + if (pmd_none(*vmf->pmd) && !vmf->prealloc_pte) { > > + vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm, vmf->address); > > + if (!vmf->prealloc_pte) > > + return VM_FAULT_OOM; > > + smp_wmb(); /* See comment in __pte_alloc() */ > > + } > > Could you be more specific in the deadlock comment? git blame will > work fine for a while, but it becomes a pain to find corresponding > patches after stuff gets moved around for years. > > In particular the race diagram between reclaim with a page lock held > and the fs doing SetPageWriteback batches before kicking off IO would > be useful directly in the code, IMO. This? diff --git a/mm/memory.c b/mm/memory.c index bb78e90a9b70..ece221e4da6d 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2995,7 +2995,18 @@ static vm_fault_t __do_fault(struct vm_fault *vmf) /* * Preallocate pte before we take page_lock because this might lead to - * deadlocks for memcg reclaim which waits for pages under writeback. + * deadlocks for memcg reclaim which waits for pages under writeback: + * lock_page(A) + * SetPageWriteback(A) + * unlock_page(A) + * lock_page(B) + * lock_page(B) + * pte_alloc_pne + * shrink_page_list + * wait_on_page_writeback(A) + * SetPageWriteback(B) + * unlock_page(B) + * # flush A, B to clear the writeback */ if (pmd_none(*vmf->pmd) && !vmf->prealloc_pte) { vmf->prealloc_pte = pte_alloc_one(vmf->vma->vm_mm, vmf->address); -- Michal Hocko SUSE Labs