From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752356AbdKJJBC (ORCPT ); Fri, 10 Nov 2017 04:01:02 -0500 Received: from verein.lst.de ([213.95.11.211]:43056 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751253AbdKJJBB (ORCPT ); Fri, 10 Nov 2017 04:01:01 -0500 Date: Fri, 10 Nov 2017 10:01:00 +0100 From: Christoph Hellwig To: Dan Williams Cc: akpm@linux-foundation.org, linux-mm@kvack.org, Christoph Hellwig , stable@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] mm: introduce get_user_pages_longterm Message-ID: <20171110090100.GA4895@lst.de> References: <151001623063.16354.14661493921524115663.stgit@dwillia2-desk3.amr.corp.intel.com> <151001623591.16354.4902423177617232098.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <151001623591.16354.4902423177617232098.stgit@dwillia2-desk3.amr.corp.intel.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > +long get_user_pages_longterm(unsigned long start, unsigned long nr_pages, > + unsigned int gup_flags, struct page **pages, > + struct vm_area_struct **vmas) > +{ > + struct vm_area_struct **__vmas = vmas; How about calling the vma argument vma_arg, and the one used vma to make thigns a little more readable? > + struct vm_area_struct *vma_prev = NULL; > + long rc, i; > + > + if (!pages) > + return -EINVAL; > + > + if (!vmas && IS_ENABLED(CONFIG_FS_DAX)) { > + __vmas = kzalloc(sizeof(struct vm_area_struct *) * nr_pages, > + GFP_KERNEL); > + if (!__vmas) > + return -ENOMEM; > + } > + > + rc = get_user_pages(start, nr_pages, gup_flags, pages, __vmas); > + > + /* skip scan for fs-dax vmas if they are compile time disabled */ > + if (!IS_ENABLED(CONFIG_FS_DAX)) > + goto out; Instead of all this IS_ENABLED magic I'd recomment to just conditionally compile this function and define it to get_user_pages in the header if FS_DAX is disabled. Else this looks fine to me. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 10 Nov 2017 10:01:00 +0100 From: Christoph Hellwig To: Dan Williams Cc: akpm@linux-foundation.org, linux-mm@kvack.org, Christoph Hellwig , stable@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] mm: introduce get_user_pages_longterm Message-ID: <20171110090100.GA4895@lst.de> References: <151001623063.16354.14661493921524115663.stgit@dwillia2-desk3.amr.corp.intel.com> <151001623591.16354.4902423177617232098.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <151001623591.16354.4902423177617232098.stgit@dwillia2-desk3.amr.corp.intel.com> Sender: owner-linux-mm@kvack.org List-ID: > +long get_user_pages_longterm(unsigned long start, unsigned long nr_pages, > + unsigned int gup_flags, struct page **pages, > + struct vm_area_struct **vmas) > +{ > + struct vm_area_struct **__vmas = vmas; How about calling the vma argument vma_arg, and the one used vma to make thigns a little more readable? > + struct vm_area_struct *vma_prev = NULL; > + long rc, i; > + > + if (!pages) > + return -EINVAL; > + > + if (!vmas && IS_ENABLED(CONFIG_FS_DAX)) { > + __vmas = kzalloc(sizeof(struct vm_area_struct *) * nr_pages, > + GFP_KERNEL); > + if (!__vmas) > + return -ENOMEM; > + } > + > + rc = get_user_pages(start, nr_pages, gup_flags, pages, __vmas); > + > + /* skip scan for fs-dax vmas if they are compile time disabled */ > + if (!IS_ENABLED(CONFIG_FS_DAX)) > + goto out; Instead of all this IS_ENABLED magic I'd recomment to just conditionally compile this function and define it to get_user_pages in the header if FS_DAX is disabled. Else this looks fine to me. -- 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