Received: by 2002:a25:31c3:0:0:0:0:0 with SMTP id x186csp719977ybx; Wed, 6 Nov 2019 07:26:27 -0800 (PST) X-Google-Smtp-Source: APXvYqyrkDVNaId80vcYWj5gPcDIfdgnbSkYXjM/Dwm+r4tjXtA91uJHQ0XKEjBwRwdbstUWPqmS X-Received: by 2002:a17:906:6d87:: with SMTP id h7mr35637656ejt.4.1573053987103; Wed, 06 Nov 2019 07:26:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1573053987; cv=none; d=google.com; s=arc-20160816; b=kPqIeS11MwoaGBOPP+xuf2MsGtUtNurYxrE+DQWRl+J6E9EZqA0NFKBkfcA3M2r+Bf ulxp64CBb54J3t5ShYewCZUP/lq+wbqnuhwUS0J1RGkXao7DoCjxpUrPpTcCm027dall no+WGn4PbNGQ3Dh6aBQzU+giqmgRYxl3CmfpbiiYaSZUu8UbDIeBhrO4TBaWf+SwAsNj 4sQBFvh7TuJjVQOF1crzYGW9Q8pY+qkOKm6gRLbJ1/HlQ/m4t+pQRFWOSa/xmRnYA4ar JxrPulWt/xMBmKGg9RcFsLAGZb8Ciqq6TJr28hbYfake2g4P/QCfkcuz+dO8Asr/t9aV eY8w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=DaIqSwh3pHkaj3mZv6F4OVGrXPdwAsMptpDm4aMJJWM=; b=HViHuy/qATUN19F7zrHcd2u+1ON1zsKneRrARpqh48zPXTjFomD2KFioBhORCb49/b GpdQlyGGlZuvKB6T+/GOE4riB4RqzuCqHiQ9rCrATffvsNxZezyoT+mK38YpCdV/2GgU sU0jOdtEYmiVGtZ8B5uqCI8mXxRuY002aeEwIlqzx1zE7/UziVJ2EEpFwKOkS986Q17C COUXz4hzYNHFd7fyV7UMsWNSkOI/FpZYG0+3DbOSs/T4n9z3kdQGKuhsxCznLuxAp7Ae JHYPWVvYOf0yKGakj49NANX5uWub1ihMxWQxGMuauBVlFL/l5w3hM+qUJS76XgSSYzZ0 IGxg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id s40si12542117edd.233.2019.11.06.07.26.03; Wed, 06 Nov 2019 07:26:27 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732098AbfKFPZH (ORCPT + 99 others); Wed, 6 Nov 2019 10:25:07 -0500 Received: from mx2.suse.de ([195.135.220.15]:50548 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728289AbfKFPZG (ORCPT ); Wed, 6 Nov 2019 10:25:06 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8E2E3B012; Wed, 6 Nov 2019 15:25:04 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id A12DC1E4353; Wed, 6 Nov 2019 16:25:03 +0100 (CET) Date: Wed, 6 Nov 2019 16:25:03 +0100 From: Jan Kara To: Josef Bacik Cc: Jan Kara , snazy@snazy.de, Johannes Weiner , Vlastimil Babka , Michal Hocko , "Kirill A. Shutemov" , Randy Dunlap , linux-kernel@vger.kernel.org, Linux MM , Andrew Morton , "Potyra, Stefan" Subject: Re: mlockall(MCL_CURRENT) blocking infinitely Message-ID: <20191106152503.GA12685@quack2.suse.cz> References: <20191025140029.GL17610@dhcp22.suse.cz> <20191105182211.GA33242@cmpxchg.org> <20191106120315.GF16085@quack2.suse.cz> <4edf4dea97f6c1e3c7d4fed0e12c3dc6dff7575f.camel@gmx.de> <20191106145608.ucvuwsuyijvkxz22@macbook-pro-91.dhcp.thefacebook.com> <20191106150524.GL16085@quack2.suse.cz> <20191106151429.swqtq2dt4uelhjzn@macbook-pro-91.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191106151429.swqtq2dt4uelhjzn@macbook-pro-91.dhcp.thefacebook.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 06-11-19 10:14:31, Josef Bacik wrote: > On Wed, Nov 06, 2019 at 04:05:24PM +0100, Jan Kara wrote: > > On Wed 06-11-19 09:56:09, Josef Bacik wrote: > > > On Wed, Nov 06, 2019 at 02:45:43PM +0100, Robert Stupp wrote: > > > > On Wed, 2019-11-06 at 13:03 +0100, Jan Kara wrote: > > > > > On Tue 05-11-19 13:22:11, Johannes Weiner wrote: > > > > > > What I don't quite understand yet is why the fault path doesn't > > > > > > make > > > > > > progress eventually. We must drop the mmap_sem without changing the > > > > > > state in any way. How can we keep looping on the same page? > > > > > > > > > > That may be a slight suboptimality with Josef's patches. If the page > > > > > is marked as PageReadahead, we always drop mmap_sem if we can and > > > > > start > > > > > readahead without checking whether that makes sense or not in > > > > > do_async_mmap_readahead(). OTOH page_cache_async_readahead() then > > > > > clears > > > > > PageReadahead so the only way how I can see we could loop like this > > > > > is when > > > > > file->ra->ra_pages is 0. Not sure if that's what's happening through. > > > > > We'd > > > > > need to find which of the paths in filemap_fault() calls > > > > > maybe_unlock_mmap_for_io() to tell more. > > > > > > > > Yes, ra_pages==0 > > > > Attached the dmesg + smaps outputs > > > > > > > > > > > > > > Ah ok I see what's happening, __get_user_pages() returns 0 if we get an EBUSY > > > from faultin_page, and then __mm_populate does nend = nstart + ret * PAGE_SIZE, > > > which just leaves us where we are. > > > > > > We need to handle the non-blocking and the locking separately in __mm_populate > > > so we know what's going on. Jan's fix for the readahead thing is definitely > > > valid as well, but this will keep us from looping forever in other retry cases. > > > > I don't think this will work. AFAICS faultin_page() just checks whether > > 'nonblocking' is != NULL but doesn't ever look at its value... Honestly the > > whole interface is rather weird like lots of things around gup(). > > > > Oh what the hell, yeah this is super bonkers. The whole fault path probably > should be cleaned up to handle retry better. This will do the trick I think? Yes, this should do it. But then I'm not sure if using __get_user_pages_locked() instead of __get_user_pages() in populate_vma_page_range() isn't a better option. I guess I'll defer that decision to mm people to pick their poison ;)... Honza > diff --git a/mm/gup.c b/mm/gup.c > index 8f236a335ae9..2468789298e6 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -628,7 +628,7 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma, > fault_flags |= FAULT_FLAG_WRITE; > if (*flags & FOLL_REMOTE) > fault_flags |= FAULT_FLAG_REMOTE; > - if (nonblocking) > + if (nonblocking && *nonblocking != 0) > fault_flags |= FAULT_FLAG_ALLOW_RETRY; > if (*flags & FOLL_NOWAIT) > fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT; > @@ -1237,6 +1237,7 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > unsigned long end, nstart, nend; > struct vm_area_struct *vma = NULL; > int locked = 0; > + int nonblocking = 1; > long ret = 0; > > end = start + len; > @@ -1268,7 +1269,7 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > * double checks the vma flags, so that it won't mlock pages > * if the vma was already munlocked. > */ > - ret = populate_vma_page_range(vma, nstart, nend, &locked); > + ret = populate_vma_page_range(vma, nstart, nend, &nonblocking); > if (ret < 0) { > if (ignore_errors) { > ret = 0; > @@ -1276,6 +1277,14 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) > } > break; > } > + > + /* > + * We dropped the mmap_sem, so we need to re-lock, and the next > + * loop around we won't drop because nonblocking is now 0. > + */ > + if (!nonblocking) > + locked = 0; > + > nend = nstart + ret * PAGE_SIZE; > ret = 0; > } -- Jan Kara SUSE Labs, CR