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 C6436C04A68 for ; Thu, 28 Jul 2022 06:51:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 49B516B0071; Thu, 28 Jul 2022 02:51:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 44ABF6B0072; Thu, 28 Jul 2022 02:51:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 312448E0001; Thu, 28 Jul 2022 02:51:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 1F3D16B0071 for ; Thu, 28 Jul 2022 02:51:45 -0400 (EDT) Received: from smtpin31.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id E35E91C5ACB for ; Thu, 28 Jul 2022 06:51:44 +0000 (UTC) X-FDA: 79735588128.31.F138B3A Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by imf09.hostedemail.com (Postfix) with ESMTP id 2B9111400C1 for ; Thu, 28 Jul 2022 06:51:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658991104; x=1690527104; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=Wj7zPRz9hbcwp/KopmE11CExQ4AFDuVqU4Mck3ExkwU=; b=jJmfn94cWqhTEY/Hv9iF880Ls4+N8z72FyBzLM0+usEmCcWMhVkTuJ2Q o5LfdzS9KBUbUpLuBEuyZ1RyTvht29N1XxyPAObU5QtkWgef9rBuGIYua RhqF6OfiT+1ssCOfxxm6/iDRpN8jfy+sNzOkoflf1mZW7A0pwSxZ84OTq UNv0zvN9rL1hfc3nqURXb8tgAyuereQ+A+KXI6EFgXB0Ixd2yoN+8bUuG g4V1B/W/gOXkWIcA2iL5/BdjywG/u0VYsUYSs2+2GYJmcmjLU/k0qeyra yHun7UhV/rp9rvQwiDZbM4nd8Ql+CBWKImHFzCsniSpuXGcWoEeDT8gZn A==; X-IronPort-AV: E=McAfee;i="6400,9594,10421"; a="289206985" X-IronPort-AV: E=Sophos;i="5.93,196,1654585200"; d="scan'208";a="289206985" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jul 2022 23:51:42 -0700 X-IronPort-AV: E=Sophos;i="5.93,196,1654585200"; d="scan'208";a="576326899" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.239.13.94]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jul 2022 23:51:38 -0700 From: "Huang, Ying" To: "Aneesh Kumar K.V" Cc: linux-mm@kvack.org, akpm@linux-foundation.org, Wei Xu , Yang Shi , Davidlohr Bueso , Tim C Chen , Michal Hocko , Linux Kernel Mailing List , Hesham Almatary , Dave Hansen , Jonathan Cameron , Alistair Popple , Dan Williams , Johannes Weiner , jvgediya.oss@gmail.com Subject: Re: [PATCH v10 5/8] mm/demotion: Build demotion targets based on explicit memory tiers References: <20220720025920.1373558-1-aneesh.kumar@linux.ibm.com> <20220720025920.1373558-6-aneesh.kumar@linux.ibm.com> <871qu8wc6c.fsf@yhuang6-desk2.ccr.corp.intel.com> <9f14814c-cb08-8032-caff-edcd0594ad41@linux.ibm.com> <87h733uyc8.fsf@yhuang6-desk2.ccr.corp.intel.com> <87k07zrx3t.fsf@linux.ibm.com> Date: Thu, 28 Jul 2022 14:51:34 +0800 In-Reply-To: <87k07zrx3t.fsf@linux.ibm.com> (Aneesh Kumar K. V.'s message of "Wed, 27 Jul 2022 10:05:50 +0530") Message-ID: <87y1wdn30p.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=jJmfn94c; spf=pass (imf09.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.24 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658991104; a=rsa-sha256; cv=none; b=2RKvpVFAwuNxy2TiMTJa7EHkUfR3HbT60/083+XXLNAyi1IMx0YxRvNXsdpPtNetNtgOqr ckDqmuC+iTb/3l8sdirxtzN9tsPvbLB0KtwjzqdqwoYW8CejgkdL5gLwApafrnejo8woGh 6i3gRakjVbVBgBaj3oCLrr0k1Tkmy4U= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658991104; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9aJJpzTAtVUBqIBKl6STn42iVOW7WGlbzDg8FY/esNE=; b=dlvjomVD5Gb5z3YdaUMunca9TUtxkZ7qOa33H9ruOpL5ftFPGxOUpSRixE91O3NCJNI+MY WEVZmxiRHRyuwmQLz0nWheIHB4T4xXT17Of8HdleK5BU2v/G4ulsCYwGAp/XUAa+1qhxDr /7Wdi3wyjKW9rvOJpYJLhalRCqL+XBI= X-Rspamd-Server: rspam10 X-Rspam-User: Authentication-Results: imf09.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=jJmfn94c; spf=pass (imf09.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.24 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Stat-Signature: 6gr1syf7aqq3xfyftd85hdkxtax3zu96 X-Rspamd-Queue-Id: 2B9111400C1 X-HE-Tag: 1658991103-461562 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: "Aneesh Kumar K.V" writes: > "Huang, Ying" writes: > >> Aneesh Kumar K V writes: >> >>> On 7/26/22 1:14 PM, Huang, Ying wrote: >>>> "Aneesh Kumar K.V" writes: >>>> > > .... > >>>> + */ >>>>> +int next_demotion_node(int node) >>>>> +{ >>>>> + struct demotion_nodes *nd; >>>>> + int target; >>>>> + >>>>> + if (!node_demotion) >>>>> + return NUMA_NO_NODE; >>>>> + >>>>> + nd = &node_demotion[node]; >>>>> + >>>>> + /* >>>>> + * node_demotion[] is updated without excluding this >>>>> + * function from running. >>>>> + * >>>>> + * Make sure to use RCU over entire code blocks if >>>>> + * node_demotion[] reads need to be consistent. >>>>> + */ >>>>> + rcu_read_lock(); >>>>> + /* >>>>> + * If there are multiple target nodes, just select one >>>>> + * target node randomly. >>>>> + * >>>>> + * In addition, we can also use round-robin to select >>>>> + * target node, but we should introduce another variable >>>>> + * for node_demotion[] to record last selected target node, >>>>> + * that may cause cache ping-pong due to the changing of >>>>> + * last target node. Or introducing per-cpu data to avoid >>>>> + * caching issue, which seems more complicated. So selecting >>>>> + * target node randomly seems better until now. >>>>> + */ >>>>> + target = node_random(&nd->preferred); >>>> >>>> In one of the most common cases, nodes_weight(&nd->preferred) == 1. >>>> Where, get_random_int() in node_random() just wastes CPU cycles and >>>> random entropy. So the original struct demotion_nodes implementation >>>> appears better. >>>> >>>> struct demotion_nodes { >>>> unsigned short nr; >>>> short nodes[DEMOTION_TARGET_NODES]; >>>> }; >>>> >>> >>> >>> Is that measurable difference? using nodemask_t makes it much easier with respect to >>> implementation. IMHO if we observe the usage of node_random() to have performance impact >>> with nodes_weight() == 1 we should fix node_random() to handle that? If you strongly >>> feel we should fix this, i can opencode node_random to special case node_weight() == 1? >> >> If there's no much difference, why not just use the existing code? >> IMHO, it's your responsibility to prove your new implementation is >> better via numbers, for example, reduced code lines, with better or same >> performance. >> >> Another policy is just to use the existing code in the first version. >> Then change it based on measurement. > > One of the reason I switched to nodemask_t is to make code simpler. > demotion target is essentially a node mask. > >> >> In general, I care more about the most common cases, that is, 0 or 1 >> demotion target. > > How about I switch to the below opencoded version. That should take care > of the above concern. Per my estimation, the performance for 0 or 1 demotion target should be OK. And I think that you can change node_random() implementation directly. Because it will not hurt other users too. Best Regards, Huang, Ying >> >>> - target = node_random(&nd->preferred); >>> + node_weight = nodes_weight(nd->preferred); >>> + switch (node_weight) { >>> + case 0: >>> + target = NUMA_NO_NODE; >>> + break; >>> + case 1: >>> + target = first_node(nd->preferred); >>> + break; >>> + default: >>> + target = bitmap_ord_to_pos(nd->preferred.bits, >>> + get_random_int() % node_weight, MAX_NUMNODES); >>> + break; >>> + } >>> >>>