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 753B3C43334 for ; Wed, 8 Jun 2022 01:07:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D86B36B0071; Tue, 7 Jun 2022 21:07:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D35A26B0072; Tue, 7 Jun 2022 21:07:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFDB16B0073; Tue, 7 Jun 2022 21:07:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B1F256B0071 for ; Tue, 7 Jun 2022 21:07:30 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7F0A12094B for ; Wed, 8 Jun 2022 01:07:30 +0000 (UTC) X-FDA: 79553280660.06.C64AB35 Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) by imf11.hostedemail.com (Postfix) with ESMTP id 22FA54005C for ; Wed, 8 Jun 2022 01:07:29 +0000 (UTC) Received: by mail-lj1-f169.google.com with SMTP id e4so5134350ljl.1 for ; Tue, 07 Jun 2022 18:07:29 -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=jlNHVl/7jgVyMJatcVlkkpjGAj2zjjFK99cQwZhyRAs=; b=sHCcN18rFe8oXpm01NlcCvLeAYg1m2VOaG6r3Dnu17fc7pgyS7ytXhpnLo5gtA6LUa 6wuJkNNcmw1YS2dJ3/jDhDDzL5X2kEXGp1ODcpq9NbHLC4sYhVRS3wJUE6/dm8Y7s+rI HvyUQzgF4BYNQ9bjXiZE+bPaVZqqhtSdIxO1eRoEN0tTDohg0XUOrOKL3HKCizTHdSFB F6ILnKOPiQTu2HFr/TRmIESZvi5sF+sAW739kLOz4XuaPhwlYjiZLjnfaPbC2Fh1c5Fc uPKY4blaWmB/7Mi6AStSnbBaD1t+nKafefkyLH65PcXASjQRFEUK4yKg/7Zz3XWcORk/ zV6Q== 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=jlNHVl/7jgVyMJatcVlkkpjGAj2zjjFK99cQwZhyRAs=; b=g9AwEli7tbXztmnPzNsOKcreyWEwdT/euUlzmrXfuGyCUpxVluJnM/Ag6W+hbOpfBq 6Oe/9QFqSFo3eJzDJzIoK0sxEMgtxvIK02vXs+2DsmxCQFUc4nqjdDAeH/C3Jh5bMcW6 B2bFF7TycAuuflaZac7wxqjyO7dfK2OERKqPB3vXj8heQGLAil2ezCQzwsSZPlIrciLZ 1Gm9tzGJ/O/2UxNSCquPzySFXBIq+JqRiDkMgqNae4O0g9D8gFiWFXZb+nD0OL3J35pS B+iiJByBDU8yVwObju6LPIApFumTYrJ3lxu5cOHvTnRm1Vk1pz4Px1mZugiqh73CIiS8 QvDg== X-Gm-Message-State: AOAM531aiVrUfUYv1YiuSvEgctcIPpVkZwAPVjx5O+v1YvgsaW5b0B5d tDdSRtb3qN/Gsk+zSQSMy7sZ1hARwDYX+R7/DQ/ofg== X-Google-Smtp-Source: ABdhPJwGhbIOT1RLB2dhh2PKsPILVdFmbLPiygIAr3wuumtpgJL03KG1r0U8ZknpDOM0J/py/cy0UBBzfliEPKz3obY= X-Received: by 2002:a2e:84c9:0:b0:253:bd3e:63b3 with SMTP id q9-20020a2e84c9000000b00253bd3e63b3mr53262470ljh.350.1654650448332; Tue, 07 Jun 2022 18:07:28 -0700 (PDT) MIME-Version: 1.0 References: <20220604004004.954674-4-zokeefe@google.com> <202206060911.I8rRqGwC-lkp@intel.com> <20220606152333.6f06f2e23a1161e444fa0f8d@linux-foundation.org> In-Reply-To: From: "Zach O'Keefe" Date: Tue, 7 Jun 2022 18:06:51 -0700 Message-ID: Subject: Re: [PATCH v6 03/15] mm/khugepaged: add struct collapse_control To: Yang Shi Cc: Andrew Morton , kernel test robot , Alex Shi , David Hildenbrand , David Rientjes , Matthew Wilcox , Michal Hocko , Pasha Tatashin , Peter Xu , Rongwei Wang , SeongJae Park , Song Liu , Vlastimil Babka , Zi Yan , Linux MM , kbuild-all@lists.01.org, Andrea Arcangeli , 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 Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: a6qhe17ugt9uyf8gsobd8bintupxjodq X-Rspam-User: Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=sHCcN18r; spf=pass (imf11.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.169 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 22FA54005C X-HE-Tag: 1654650449-597425 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 Tue, Jun 7, 2022 at 6:00 PM Yang Shi wrote: > > On Tue, Jun 7, 2022 at 5:43 PM Zach O'Keefe wrote: > > > > On Mon, Jun 6, 2022 at 4:54 PM Yang Shi wrote: > > > > > > On Mon, Jun 6, 2022 at 3:23 PM Andrew Morton wrote: > > > > > > > > On Mon, 6 Jun 2022 09:40:20 -0700 "Zach O'Keefe" wrote: > > > > > > > > > On Sun, Jun 5, 2022 at 7:42 PM kernel test robot wrote: > > > > > > > > > > > > Hi Zach, > > > > > > > > > > > > Thank you for the patch! Perhaps something to improve: > > > > > > > > > > > > [auto build test WARNING on akpm-mm/mm-everything] > > > > > > > > > > > > url: https://github.com/intel-lab-lkp/linux/commits/Zach-O-Keefe/mm-userspace-hugepage-collapse/20220606-012953 > > > > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > > > > > > config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220606/202206060911.I8rRqGwC-lkp@intel.com/config) > > > > > > compiler: gcc-11 (Debian 11.3.0-1) 11.3.0 > > > > > > reproduce (this is a W=1 build): > > > > > > # https://github.com/intel-lab-lkp/linux/commit/d87b6065d6050b89930cca0814921aca7c269286 > > > > > > git remote add linux-review https://github.com/intel-lab-lkp/linux > > > > > > git fetch --no-tags linux-review Zach-O-Keefe/mm-userspace-hugepage-collapse/20220606-012953 > > > > > > git checkout d87b6065d6050b89930cca0814921aca7c269286 > > > > > > # save the config file > > > > > > mkdir build_dir && cp config build_dir/.config > > > > > > make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash > > > > > > > > > > > > If you fix the issue, kindly add following tag where applicable > > > > > > Reported-by: kernel test robot > > > > > > > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > > > > > mm/khugepaged.c: In function 'khugepaged': > > > > > > >> mm/khugepaged.c:2284:1: warning: the frame size of 4160 bytes is larger than 2048 bytes [-Wframe-larger-than=] > > > > > > 2284 | } > > > > > > | ^ > > > > > > > > > > Thanks lkp@intel.com. > > > > > > > > > > This is due to config with: > > > > > > > > > > CONFIG_FRAME_WARN=2048 > > > > > CONFIG_NODES_SHIFT=10 > > > > > > > > > > Where struct collapse_control has a member int > > > > > node_load[MAX_NUMNODES], and we stack allocate one. > > > > > > > > > > Is this a configuration that needs to be supported? 1024 nodes seems > > > > > like a lot and I'm not sure if these configs are randomly generated or > > > > > are reminiscent of real systems. > > > > > > > > Adding 4k to the stack isn't a good thing to do. It's trivial to > > > > kmalloc the thing, so why not do that? > > > > > > Thanks, Andrew. Yeah, I just suggested that too. > > > > Thanks Yang / Andrew for taking the time to voice your suggestions. > > > > I'll go ahead and just kmalloc() the thing and fail if we can't. > > > > Yang, is there a reason to kmalloc() the entire struct > > collapse_control with trailing flex array vs stack allocating the > > struct collapse_control + kmalloc()'ing the node_load array? > > I don't think those two have too much difference. I don't have a > strong preference personally. However you could choose: > > Define collapse_control as: > struct collapse_control { > xxx; > ... > int node_load[MAX_NUMANODES]; > } > Then you could kmalloc the whole struct. > > Or it could be defined as: > struct collapse_control { > xxx; > ... > int *node_load[]; > } > In this way you could allocate collapse_control on stack or by > kmalloc, then kmalloc node_load for all possible nodes instead of > MAX_NUMANODES. This may have a better success rate since you do > kmalloc much less memory (typically the number of possible nodes is > much less than MAX_NUMANODES), but it may be not worth it since the > error handling path is more complicated and it may not make too much > difference. > > The first choice is definitely much simpler, you may want to try that first. Thanks for the suggestion. First approach also has the benefit of being able to statically allocate one for khugepaged and simplifies error paths there. I'll try that. Again, thanks for taking the time to review and help out / suggest improvements :) Best, Zach > > > > > > > > > > > > I'll await some reviewer input (hopefully positive ;)) before merging > > > > this series.