From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Hansen Subject: Re: [PATCHv4 15/39] thp, mm: trigger bug in replace_page_cache_page() on THP Date: Tue, 28 May 2013 09:33:35 -0700 Message-ID: <51A4DC5F.7050406@sr71.net> References: <1368321816-17719-1-git-send-email-kirill.shutemov@linux.intel.com> <1368321816-17719-16-git-send-email-kirill.shutemov@linux.intel.com> <519BD65C.1050709@sr71.net> <20130528125328.5385CE0090@blue.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Andrea Arcangeli , Andrew Morton , Al Viro , Hugh Dickins , Wu Fengguang , Jan Kara , Mel Gorman , linux-mm@kvack.org, Andi Kleen , Matthew Wilcox , "Kirill A. Shutemov" , Hillf Danton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: "Kirill A. Shutemov" Return-path: In-Reply-To: <20130528125328.5385CE0090@blue.fi.intel.com> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On 05/28/2013 05:53 AM, Kirill A. Shutemov wrote: > Dave Hansen wrote: >> On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote: >>> + VM_BUG_ON(PageTransHuge(old)); >>> + VM_BUG_ON(PageTransHuge(new)); >>> VM_BUG_ON(!PageLocked(old)); >>> VM_BUG_ON(!PageLocked(new)); >>> VM_BUG_ON(new->mapping); >> >> The code calling replace_page_cache_page() has a bunch of fallback and >> error returning code. It seems a little bit silly to bring the whole >> machine down when you could just WARN_ONCE() and return an error code >> like fuse already does: > > What about: > > if (WARN_ONCE(PageTransHuge(old) || PageTransHuge(new), > "%s: unexpected huge page\n", __func__)) > return -EINVAL; That looks sane to me. But, please do make sure to differentiate in the error message between thp and hugetlbfs (if you have the room). BTW, I'm also not sure you need to print the function name. The WARN_ON() register dump usually has the function name. -- 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 S934840Ab3E1Qdl (ORCPT ); Tue, 28 May 2013 12:33:41 -0400 Received: from www.sr71.net ([198.145.64.142]:37468 "EHLO blackbird.sr71.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934675Ab3E1Qdj (ORCPT ); Tue, 28 May 2013 12:33:39 -0400 Message-ID: <51A4DC5F.7050406@sr71.net> Date: Tue, 28 May 2013 09:33:35 -0700 From: Dave Hansen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: "Kirill A. Shutemov" CC: Andrea Arcangeli , Andrew Morton , Al Viro , Hugh Dickins , Wu Fengguang , Jan Kara , Mel Gorman , linux-mm@kvack.org, Andi Kleen , Matthew Wilcox , "Kirill A. Shutemov" , Hillf Danton , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv4 15/39] thp, mm: trigger bug in replace_page_cache_page() on THP References: <1368321816-17719-1-git-send-email-kirill.shutemov@linux.intel.com> <1368321816-17719-16-git-send-email-kirill.shutemov@linux.intel.com> <519BD65C.1050709@sr71.net> <20130528125328.5385CE0090@blue.fi.intel.com> In-Reply-To: <20130528125328.5385CE0090@blue.fi.intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/28/2013 05:53 AM, Kirill A. Shutemov wrote: > Dave Hansen wrote: >> On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote: >>> + VM_BUG_ON(PageTransHuge(old)); >>> + VM_BUG_ON(PageTransHuge(new)); >>> VM_BUG_ON(!PageLocked(old)); >>> VM_BUG_ON(!PageLocked(new)); >>> VM_BUG_ON(new->mapping); >> >> The code calling replace_page_cache_page() has a bunch of fallback and >> error returning code. It seems a little bit silly to bring the whole >> machine down when you could just WARN_ONCE() and return an error code >> like fuse already does: > > What about: > > if (WARN_ONCE(PageTransHuge(old) || PageTransHuge(new), > "%s: unexpected huge page\n", __func__)) > return -EINVAL; That looks sane to me. But, please do make sure to differentiate in the error message between thp and hugetlbfs (if you have the room). BTW, I'm also not sure you need to print the function name. The WARN_ON() register dump usually has the function name.