Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754968AbdGXPYI (ORCPT ); Mon, 24 Jul 2017 11:24:08 -0400 Received: from mga04.intel.com ([192.55.52.120]:62412 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751671AbdGXPX7 (ORCPT ); Mon, 24 Jul 2017 11:23:59 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,407,1496127600"; d="scan'208";a="131160790" Date: Mon, 24 Jul 2017 09:23:57 -0600 From: Ross Zwisler To: Jan Kara Cc: Ross Zwisler , Andrew Morton , linux-kernel@vger.kernel.org, "Darrick J. Wong" , "Theodore Ts'o" , Alexander Viro , Andreas Dilger , Christoph Hellwig , Dan Williams , Dave Chinner , David Airlie , Ingo Molnar , Inki Dae , Jonathan Corbet , Joonyoung Shim , Krzysztof Kozlowski , Kukjin Kim , Kyungmin Park , Matthew Wilcox , Patrik Jakobsson , Rob Clark , Seung-Woo Kim , Steven Rostedt , Tomi Valkeinen , dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-doc@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@lists.01.org, linux-samsung-soc@vger.kernel.org, linux-xfs@vger.kernel.org Subject: Re: [PATCH v4 1/5] mm: add mkwrite param to vm_insert_mixed() Message-ID: <20170724152357.GB1639@linux.intel.com> Mail-Followup-To: Ross Zwisler , Jan Kara , Andrew Morton , linux-kernel@vger.kernel.org, "Darrick J. Wong" , Theodore Ts'o , Alexander Viro , Andreas Dilger , Christoph Hellwig , Dan Williams , Dave Chinner , David Airlie , Ingo Molnar , Inki Dae , Jonathan Corbet , Joonyoung Shim , Krzysztof Kozlowski , Kukjin Kim , Kyungmin Park , Matthew Wilcox , Patrik Jakobsson , Rob Clark , Seung-Woo Kim , Steven Rostedt , Tomi Valkeinen , dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-doc@vger.kernel.org, linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-nvdimm@lists.01.org, linux-samsung-soc@vger.kernel.org, linux-xfs@vger.kernel.org References: <20170721223956.29485-1-ross.zwisler@linux.intel.com> <20170721223956.29485-2-ross.zwisler@linux.intel.com> <20170724112530.GI652@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170724112530.GI652@quack2.suse.cz> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1942 Lines: 48 On Mon, Jul 24, 2017 at 01:25:30PM +0200, Jan Kara wrote: > > @@ -1658,14 +1658,28 @@ static int insert_pfn(struct vm_area_struct *vma, unsigned long addr, > > if (!pte) > > goto out; > > retval = -EBUSY; > > - if (!pte_none(*pte)) > > - goto out_unlock; > > + if (!pte_none(*pte)) { > > + if (mkwrite) { > > + if (WARN_ON_ONCE(pte_pfn(*pte) != pfn_t_to_pfn(pfn))) > > Is the WARN_ON_ONCE() really appropriate here? Your testcase with private > mappings has triggered this situation if I'm right... Yep, I think this WARN_ON_ONCE() is correct. The test with private mappings had collisions between read-only DAX mappings which were being faulted in via insert_pfn(), and read/write COW page cache mappings which were being faulted in by wp_page_copy(). I was hitting a false-positive warning when I had the WARN_ON_ONCE() in insert_pfn() outside of the mkwrite case, i.e.: if (!pte_none(*pte)) { if (WARN_ON_ONCE(pte_pfn(*pte) != pfn_t_to_pfn(pfn))) goto out_unlock; if (mkwrite) { entry = *pte; goto out_mkwrite; } else goto out_unlock; } This was triggering when one thread was faulting in a read-only DAX mapping when another thread had already faulted in a read-write COW page cache page. The patches I sent out have the warning in the mkwrite case, which would mean that we were getting a fault for a read/write PTE in insert_pfn() and the PFN didn't match what was already in the PTE. This can't ever happen in the private mapping case because we will never install a read/write PTE for normal storage, only for COW page cache pages. Essentially I don't think we should ever be able to hit this warning, and if we do I'd like to get the bug report so that I can track down how it was happening and make sure that it's safe. It is in the mkwrite path of insert_pfn() which is currently only used by the DAX code. Does that make sense to you, or would you recommend leaving it out? (If so, why?)