From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 2 Sep 2015 12:47:39 +0300 From: "Kirill A. Shutemov" Subject: Re: [PATCH] dax, pmem: add support for msync Message-ID: <20150902094739.GA2627@node.dhcp.inet.fi> References: <1441047584-14664-1-git-send-email-ross.zwisler@linux.intel.com> <20150831233803.GO3902@dastard> <20150901100804.GA7045@node.dhcp.inet.fi> <20150901224922.GR3902@dastard> <20150902091321.GA2323@node.dhcp.inet.fi> <55E6C36C.3090402@plexistor.com> <55E6C458.3040901@plexistor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55E6C458.3040901@plexistor.com> Sender: owner-linux-mm@kvack.org To: Boaz Harrosh Cc: Dave Chinner , Andrew Morton , x86@kernel.org, linux-nvdimm@lists.01.org, Peter Zijlstra , Dave Hansen , Hugh Dickins , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Ingo Molnar , Thomas Gleixner , Alexander Viro , "H. Peter Anvin" , linux-fsdevel@vger.kernel.org, Christoph Hellwig , "Kirill A. Shutemov" List-ID: On Wed, Sep 02, 2015 at 12:41:44PM +0300, Boaz Harrosh wrote: > On 09/02/2015 12:37 PM, Boaz Harrosh wrote: > >> > >> + /* > >> + * Make sure that for VM_MIXEDMAP VMA has both > >> + * vm_ops->page_mkwrite and vm_ops->pfn_mkwrite or has none. > >> + */ > >> + if ((vma->vm_ops->page_mkwrite || vma->vm_ops->pfn_mkwrite) && > >> + vma->vm_flags & VM_MIXEDMAP) { > >> + VM_BUG_ON_VMA(!vma->vm_ops->page_mkwrite, vma); > >> + VM_BUG_ON_VMA(!vma->vm_ops->pfn_mkwrite, vma); > > > > BTW: the page_mkwrite is used for reading of holes that put zero-pages at the radix tree. > > One can just map a single global zero-page in pfn-mode for that. > > > > Kirill Hi. Please don't make these BUG_ONs its counter productive believe me. This is VM_BUG_ON, not normal BUG_ON. VM_BUG_ON is under CONFIG_DEBUG_VM which is disabled on production kernels. > > Please make them WARN_ON_ONCE() it is not a crashing bug to work like this. > > (Actually it is not a bug at all in some cases, but we can relax that when a user > > comes up) > > > > Thanks > > Boaz > > > > Second thought I do not like this patch. This is why we have xftests for, the fact of it > is that test 080 catches this. For me this is enough. I don't insist on applying the patch. And I worry about false-positives. > An FS developer should test his code, and worst case we help him on ML, like we did > in this case. > > Thanks > Boaz > > >> + } > >> addr = vma->vm_start; > >> vm_flags = vma->vm_flags; > >> } else if (vm_flags & VM_SHARED) { > >> > > > -- Kirill A. Shutemov -- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753234AbbIBJro (ORCPT ); Wed, 2 Sep 2015 05:47:44 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:34139 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752809AbbIBJrm (ORCPT ); Wed, 2 Sep 2015 05:47:42 -0400 Date: Wed, 2 Sep 2015 12:47:39 +0300 From: "Kirill A. Shutemov" To: Boaz Harrosh Cc: Dave Chinner , Andrew Morton , x86@kernel.org, linux-nvdimm@ml01.01.org, Peter Zijlstra , Dave Hansen , Hugh Dickins , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Ingo Molnar , Thomas Gleixner , Alexander Viro , "H. Peter Anvin" , linux-fsdevel@vger.kernel.org, Christoph Hellwig , "Kirill A. Shutemov" Subject: Re: [PATCH] dax, pmem: add support for msync Message-ID: <20150902094739.GA2627@node.dhcp.inet.fi> References: <1441047584-14664-1-git-send-email-ross.zwisler@linux.intel.com> <20150831233803.GO3902@dastard> <20150901100804.GA7045@node.dhcp.inet.fi> <20150901224922.GR3902@dastard> <20150902091321.GA2323@node.dhcp.inet.fi> <55E6C36C.3090402@plexistor.com> <55E6C458.3040901@plexistor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55E6C458.3040901@plexistor.com> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 02, 2015 at 12:41:44PM +0300, Boaz Harrosh wrote: > On 09/02/2015 12:37 PM, Boaz Harrosh wrote: > >> > >> + /* > >> + * Make sure that for VM_MIXEDMAP VMA has both > >> + * vm_ops->page_mkwrite and vm_ops->pfn_mkwrite or has none. > >> + */ > >> + if ((vma->vm_ops->page_mkwrite || vma->vm_ops->pfn_mkwrite) && > >> + vma->vm_flags & VM_MIXEDMAP) { > >> + VM_BUG_ON_VMA(!vma->vm_ops->page_mkwrite, vma); > >> + VM_BUG_ON_VMA(!vma->vm_ops->pfn_mkwrite, vma); > > > > BTW: the page_mkwrite is used for reading of holes that put zero-pages at the radix tree. > > One can just map a single global zero-page in pfn-mode for that. > > > > Kirill Hi. Please don't make these BUG_ONs its counter productive believe me. This is VM_BUG_ON, not normal BUG_ON. VM_BUG_ON is under CONFIG_DEBUG_VM which is disabled on production kernels. > > Please make them WARN_ON_ONCE() it is not a crashing bug to work like this. > > (Actually it is not a bug at all in some cases, but we can relax that when a user > > comes up) > > > > Thanks > > Boaz > > > > Second thought I do not like this patch. This is why we have xftests for, the fact of it > is that test 080 catches this. For me this is enough. I don't insist on applying the patch. And I worry about false-positives. > An FS developer should test his code, and worst case we help him on ML, like we did > in this case. > > Thanks > Boaz > > >> + } > >> addr = vma->vm_start; > >> vm_flags = vma->vm_flags; > >> } else if (vm_flags & VM_SHARED) { > >> > > > -- Kirill A. Shutemov