From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0564CA9ED0 for ; Thu, 31 Oct 2019 21:10:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9AEE621925 for ; Thu, 31 Oct 2019 21:10:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9AEE621925 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1A8A66B000A; Thu, 31 Oct 2019 17:10:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 158E66B000C; Thu, 31 Oct 2019 17:10:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 06E176B000D; Thu, 31 Oct 2019 17:10:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0023.hostedemail.com [216.40.44.23]) by kanga.kvack.org (Postfix) with ESMTP id CC1366B000A for ; Thu, 31 Oct 2019 17:10:01 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id 638818249980 for ; Thu, 31 Oct 2019 21:10:01 +0000 (UTC) X-FDA: 76105322202.18.crow10_1d0bf99748c1c X-HE-Tag: crow10_1d0bf99748c1c X-Filterd-Recvd-Size: 4242 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by imf20.hostedemail.com (Postfix) with ESMTP for ; Thu, 31 Oct 2019 21:09:58 +0000 (UTC) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Oct 2019 14:09:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,253,1569308400"; d="scan'208";a="375376881" Received: from iweiny-desk2.sc.intel.com ([10.3.52.157]) by orsmga005.jf.intel.com with ESMTP; 31 Oct 2019 14:09:55 -0700 Date: Thu, 31 Oct 2019 14:09:55 -0700 From: Ira Weiny To: John Hubbard Cc: Andrew Morton , Al Viro , Alex Williamson , Benjamin Herrenschmidt , =?iso-8859-1?Q?Bj=F6rn_T=F6pel?= , Christoph Hellwig , Dan Williams , Daniel Vetter , Dave Chinner , David Airlie , "David S . Miller" , Jan Kara , Jason Gunthorpe , Jens Axboe , Jonathan Corbet , =?iso-8859-1?B?Suly9G1l?= Glisse , Magnus Karlsson , Mauro Carvalho Chehab , Michael Ellerman , Michal Hocko , Mike Kravetz , Paul Mackerras , Shuah Khan , Vlastimil Babka , bpf@vger.kernel.org, dri-devel@lists.freedesktop.org, kvm@vger.kernel.org, linux-block@vger.kernel.org, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-media@vger.kernel.org, linux-rdma@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, netdev@vger.kernel.org, linux-mm@kvack.org, LKML , Christoph Hellwig , "Aneesh Kumar K . V" Subject: Re: [PATCH 02/19] mm/gup: factor out duplicate code from four routines Message-ID: <20191031210954.GE14771@iweiny-DESK2.sc.intel.com> References: <20191030224930.3990755-1-jhubbard@nvidia.com> <20191030224930.3990755-3-jhubbard@nvidia.com> <20191031183549.GC14771@iweiny-DESK2.sc.intel.com> <75b557f7-24b2-740c-2640-2f914d131600@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <75b557f7-24b2-740c-2640-2f914d131600@nvidia.com> User-Agent: Mutt/1.11.1 (2018-12-01) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Oct 31, 2019 at 11:43:37AM -0700, John Hubbard wrote: > On 10/31/19 11:35 AM, Ira Weiny wrote: > > On Wed, Oct 30, 2019 at 03:49:13PM -0700, John Hubbard wrote: > ... > >> + > >> +static void __remove_refs_from_head(struct page *page, int refs) > >> +{ > >> + /* Do a get_page() first, in case refs == page->_refcount */ > >> + get_page(page); > >> + page_ref_sub(page, refs); > >> + put_page(page); > >> +} > > > > I wonder if this is better implemented as "put_compound_head()"? To match the > > try_get_compound_head() call below? > > Hi Ira, > > Good idea, I'll rename it to that. > > > > >> + > >> +static int __huge_pt_done(struct page *head, int nr_recorded_pages, int *nr) > >> +{ > >> + *nr += nr_recorded_pages; > >> + SetPageReferenced(head); > >> + return 1; > > > > When will this return anything but 1? > > > > Never, but it saves a line at all four call sites, by having it return like that. > > I could see how maybe people would prefer to just have it be a void function, > and return 1 directly at the call sites. Since this was a lower line count I > thought maybe it would be slightly better, but it's hard to say really. It is a NIT perhaps but I feel like the signature of a function should stand on it's own. What this does is mix the meaning of this function with those calling it. Which IMO is not good style. We can see what others say. Ira > > thanks, > > John Hubbard > NVIDIA >