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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 02899C433F5 for ; Fri, 29 Apr 2022 16:01:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 056066B0072; Fri, 29 Apr 2022 12:01:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 004346B0073; Fri, 29 Apr 2022 12:01:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E11326B0074; Fri, 29 Apr 2022 12:01:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id D3ABF6B0072 for ; Fri, 29 Apr 2022 12:01:50 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id A5791609AC for ; Fri, 29 Apr 2022 16:01:50 +0000 (UTC) X-FDA: 79410382380.13.11C295F Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by imf19.hostedemail.com (Postfix) with ESMTP id 996671A0068 for ; Fri, 29 Apr 2022 16:01:45 +0000 (UTC) Received: by mail-lf1-f49.google.com with SMTP id n14so14800209lfu.13 for ; Fri, 29 Apr 2022 09:01:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6Y7RHvTnWUuhEDkkwMtc8bVWIk1in0671phJQ5Kw3gE=; b=Hw2sy5m+hL+7gTyvs1K+WROQouwlpcArtxFPvLqNVsd5S2U/4CnhyapvWASSC9dKKU xngC/xCb6NPwYhLb2MRdXBE1VNw+S1JycNu8IcQi3RAMU9BgrfEBgUvKOGP9Mh/6nfgA GUBp5BJzHxZIJhQoqVFV7jaPvfwYGZ0wgyzGHtt61W2n0O4eEYX6k0BdVD+hf6BMgjun KJ6+uBgeLRl8amWtpvtW+tDH72++P4G9jshWMHXpQxAACR18q1i6c7oQoX03nVY9sO8K G5jAV2pKx4+xcOoS0RlB1svm5oRQy9VD4pXmUV1FVMjfx+x2UG83dnSW40hl8tMNisTS N/dQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6Y7RHvTnWUuhEDkkwMtc8bVWIk1in0671phJQ5Kw3gE=; b=PCJP6lyVZh/8TWR/VC18imB3nJv2F7ehbu3wi1Mh9P1E8Ww0eakEgzO95Kn/6bv9Oo pjBtzwLzB0TwSBge8cN+PMogBbC5hs8KdJ7MZuDOU3NZpJ76SRbVYB32MYQ6cXTyZjWa uXW55mM25HoiUkwfUvU0d+vp7p0apKlsUaNf4TB6ablXXaOxwhQrHL43n5oI33XLT1qq pUNe/A635S4UvNIepqUfa318V9CroV/lS/2dssZl1Dn2Z8ZoHHRqlG/gVqKQPswUZumP OvDGkD5iDe5UQN961bQy3Ai5EwKeg+9WtGcRxu1O0+aLahS9KR97pxjkL2HlgDCNg68y PrKw== X-Gm-Message-State: AOAM533nx4m+83qc039+k81lg/yHXA9exfupRaRNFpihlGszxE5VD2A3 8qbIqNgEHx6esH8tDfcxZRli/o475S8npHuIpHizkA== X-Google-Smtp-Source: ABdhPJzwPneHXoUsGzKIezKTAdlfe7WSu3m5t8AgLrLtgRNE9XOpovlVYEB3xKE5pYP6/DWN89Y3eshJCoWYYMAwOwk= X-Received: by 2002:a05:6512:1109:b0:472:25d9:d262 with SMTP id l9-20020a056512110900b0047225d9d262mr11277641lfg.128.1651248108133; Fri, 29 Apr 2022 09:01:48 -0700 (PDT) MIME-Version: 1.0 References: <20220426144412.742113-1-zokeefe@google.com> <20220426144412.742113-5-zokeefe@google.com> In-Reply-To: From: "Zach O'Keefe" Date: Fri, 29 Apr 2022 09:01:10 -0700 Message-ID: Subject: Re: [PATCH v3 04/12] mm/khugepaged: add struct collapse_result To: Peter Xu Cc: Alex Shi , David Hildenbrand , David Rientjes , Matthew Wilcox , Michal Hocko , Pasha Tatashin , SeongJae Park , Song Liu , Vlastimil Babka , Yang Shi , Zi Yan , linux-mm@kvack.org, Andrea Arcangeli , Andrew Morton , Arnd Bergmann , Axel Rasmussen , Chris Kennelly , Chris Zankel , Helge Deller , Hugh Dickins , Ivan Kokshaysky , "James E.J. Bottomley" , Jens Axboe , "Kirill A. Shutemov" , Matt Turner , Max Filippov , Miaohe Lin , Minchan Kim , Patrick Xia , Pavel Begunkov , Thomas Bogendoerfer Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 996671A0068 X-Stat-Signature: xamuohux7icj3g4or885zqefyuzngkj5 Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=Hw2sy5m+; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of zokeefe@google.com designates 209.85.167.49 as permitted sender) smtp.mailfrom=zokeefe@google.com X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1651248105-719064 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, Apr 28, 2022 at 4:21 PM Peter Xu wrote: > > On Thu, Apr 28, 2022 at 02:59:44PM -0700, Zach O'Keefe wrote: > > On Wed, Apr 27, 2022 at 1:47 PM Peter Xu wrote: > > > > > > On Tue, Apr 26, 2022 at 07:44:04AM -0700, Zach O'Keefe wrote: > > > > +/* Gather information from one khugepaged_scan_[pmd|file]() request */ > > > > +struct collapse_result { > > > > + enum scan_result result; > > > > + > > > > + /* Was mmap_lock dropped during request? */ > > > > + bool dropped_mmap_lock; > > > > +}; > > > > > > IMHO this new dropped_mmap_lock makes things a bit more complicated.. > > > > > > To me, the old code actually is easy to read on when the lock is dropped: > > > > > > - For file, we always drop it > > > - For anon, when khugepaged_scan_pmd() returns 1 > > > > > > That's fairly clear to me. > > > > > > > Agreed - but I know when I first read the code it wasn't obvious to me > > what the return value of khugepaged_scan_pmd() was saying. I thought > > giving it a name ("dropped_mmap_lock") would help future readers. > > Yes that's a fair point. It's not that obvious to me too when I first read > it, but I am just worried hiding that too deep could make it complicated in > another form. > > Maybe we can rename "ret" in khugepaged_scan_mm_slot() to "lock_dropped", > or we can comment above khugepaged_scan_pmd() explaining the retval. > Or.. both? > So after continuing to play around with your suggestion for a bit (thank you again), I found that with minimal effort, we could just pipe the scan_result back through function return values. The only point where this becomes awkward is for khugepaged_scan_pmd() where the return value already has a specific meaning. Solving both problems at once, we can both (a) remove another member from struct collapse_control (result) by passing this value along as the return value of functions, and (b) move this "we dropped the lock" return value into a "bool *mmap_locked" arg to khugepaged_scan_pmd(). The resulting code rebased on this change is much cleaner overall. > -- > Peter Xu >