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 43AFAD0BB5B for ; Thu, 24 Oct 2024 03:51:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BD32D6B0088; Wed, 23 Oct 2024 23:51:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B83F26B0089; Wed, 23 Oct 2024 23:51:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A4AEA6B008A; Wed, 23 Oct 2024 23:51:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 8657C6B0088 for ; Wed, 23 Oct 2024 23:51:20 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 7AA2581026 for ; Thu, 24 Oct 2024 03:51:04 +0000 (UTC) X-FDA: 82707120468.02.80C3863 Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by imf14.hostedemail.com (Postfix) with ESMTP id 1EEE3100011 for ; Thu, 24 Oct 2024 03:50:57 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gVZ9Wxv8; spf=pass (imf14.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729741826; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gd3Srh26b/hRgJS4SPAPk1cjvKM69P2RHdfsB4GHQHk=; b=Wc/OcnlZfFwBtyhctxCMjSKfhDcz+q3FXrzKSOP1neXR04UwWzAZrFeRhDE1Rur6VE8jrj iuexd9J5xnxIyJXDMpSJyiWctVilTLVfV/yvlOT4W98F1QMfmcqm+uoevRhz38lgxoVwZ7 W/mKxKCeIegJ5nAaSyuG5gj2qpV5Om8= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=gVZ9Wxv8; spf=pass (imf14.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729741826; a=rsa-sha256; cv=none; b=3noue0xu8xIEN0U8wzR1iEjULqPsWK0Td+EXoAnM5ol/8FlUFSir0Sjm20WHujkfOjUW8D Pl6zjIXBzCHGkg6i91VITjVk9wJsOeT2bcWfRWhXbsTZizWnL9AWVtilxK/noDrWJWVBTW aNh1ZDuJdD8cgXoeSp1jSPBGqWftN5o= Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2fb5743074bso3571821fa.1 for ; Wed, 23 Oct 2024 20:51:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729741876; x=1730346676; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=gd3Srh26b/hRgJS4SPAPk1cjvKM69P2RHdfsB4GHQHk=; b=gVZ9Wxv8R50sY2DP7JAZVzFkUdi/CInomnju8lhIEQRN54dMFZlWGhDdB2rkv9JeOt V0GHN0dHjxqLFbeaTqEjHIzYrmhHoc8+EAFwSzmAeYC4vEMkj1FD3do8bQxwbgTJztSh 71LLBnwWdvUFi4Iy8J2g/DAyLEusHTNHLaCENPTu3KMJ7xEYkLFgve+u+jGOIQbZ4sUk K9GoWUZ9CgCkqoFBuxzZrFZ4WVkONUyqY1Ix9ncdUISVy8Wi8B6Yq/LU1iNnuvMkdJWQ aLsSgW0QNrWqafXQPLYLKIWXKjky36oD6qdMWfjYFl8z4uM8r7jwi9pCZWir8/24IiWd IdIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729741876; x=1730346676; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gd3Srh26b/hRgJS4SPAPk1cjvKM69P2RHdfsB4GHQHk=; b=NL2lHaKYi70Kw1NyN5tBbBiywlMZIZJpHcVNKFgtITH3HW7oXjxzHWo1vQJajuah1s C71aJNSAgl08NcScU0Qre7+2hXZK/LQ9WPLD2zp2/d5Yph7ukdO2NXI7Mi3xJ5Q6z36D sBXFzNahm0SN0NxeIwuMRFhicTaHeFJdZRDHPUpVF+mbWWAZYBWsK5m50wnzh3xS3763 p6JTrHyk+V7ecGGm9E032Po+oL5zcxPVc2Dxbxet/JslRwpZjASvhV28MLrcWCTT0ASO IL5fE08jho2p24xsgO1MxKbkdG++rj45cjSH8GIhttU4eZAOOW/Ms2yp1cam8uP8i1NQ fITg== X-Gm-Message-State: AOJu0YzpYH8NMarXc1NFVCp8/cYcdb6/B9IUHOecwCGl1XC19NOilJsK zFN+uZxSkBrDM30ddv8UfVGwo6SOM7ImhUJEWprf7tt8GVLkbBqyy/goGmJrRGqi9MzGvntQMA5 fhla33WUgYr/N7lDIaXm91/by5W0= X-Google-Smtp-Source: AGHT+IHeBYTi3bDCNFWBqdWFFSxEb/7SFE0AL9yTrkOwQAk0ssb4lbMDCKIr/VHpKzJkL/DPCVhYmQZg86LIZq/XAFM= X-Received: by 2002:a05:651c:2226:b0:2f3:f287:17df with SMTP id 38308e7fff4ca-2fca836154fmr1907561fa.20.1729741876151; Wed, 23 Oct 2024 20:51:16 -0700 (PDT) MIME-Version: 1.0 References: <871q0641xd.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: <871q0641xd.fsf@yhuang6-desk2.ccr.corp.intel.com> From: Kairui Song Date: Thu, 24 Oct 2024 11:50:59 +0800 Message-ID: Subject: Re: cluster_alloc_swap_entry() may change per-cpu cluster of another CPU randomly To: "Huang, Ying" Cc: linux-mm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: 4146dxg1hik7o5hespbadj8ma59khhic X-Rspamd-Queue-Id: 1EEE3100011 X-Rspamd-Server: rspam11 X-HE-Tag: 1729741857-947224 X-HE-Meta: U2FsdGVkX19L6jequWFKRreIexzWcJ5X0eUPd6AUqa8dCVmbtJpe/ts9berhtf48BRq/8HfTJ9WB/SSYO/Tj979t12mIQCCQwTGMWnV7ZY69rVr6v1QzrMQe9nhGgS6Lv63qYRxZ2shnv9ktHTNxv55Eddi358P+lEuObwmei+ueCfRbKphDNb971Jb5mCSxkipPeeNH/sI1zkrBFnN+akFKdqIEL1MvjM3Ss/ZbH/NP3exge1ifuz5Kgohw0arCDSKPOE4621ls0Q/cIjRrAr/Jx6OiVaY8J5E7QMIMVzbXFYj/E9YDm0CXc3TWWrjfzrUAA1LfWXpYlZSs3lpCSy2d2qPZxHxASC3843kziOJ3vy6MuemoXGQjV9U49g7T/TLuTFneVeNyF1e8Dpxn3vLqmoqZ5Gtk4MkcYMP86RT/+ghV6MWu/p3+8btl1e9DKww9nco+ENSWCzEzQ0ft/uOxl86YIDxBLloPvhoAa8GKDIHDzmixNH7ibK3Gh9f2KW0HbWLN7a1PPS1ZCymOKv2iZFlAbOD5y2tWsf1T7aYfOSW4w18yWWqt7SqWK3sTl1pPVrjp21AT5MSeqhUjG1caBhtyNFuhI1Xdyli28lsRkYRjHMUIqqorjUj7Pj7rLZ0CJJ2XzOizNesjAdxrDiXMBpPSmhUsYA7U+8/ydrmChZ5sPpl4VbHfJqux3HqzZ2UDUi3jVZsSWXU/6HqgYiAyZZ1DgGbExJl9XcdV+UaxuF0YsBRxE6r0pAitN5JMUq99XgRoDyQSMnUjYThk3qR26wtRT79zGhkfaR5wdmkt3Ha1yVz9RuSS5e4pGnsS8rpByL8gKLlnobFbD0dxJGrTC0lUXM0zW9wdB4zq00WcA1t0TZk/sdxC9wsrIGT9geOMRnlE8OyrFhhaaJAFwJbgEpfE9t93bs3zI0dqr/o3Wjr7fLOyCc17UA4aZwyNtxSkJ3L+7PLODnnnEe7 FEBGiCmX 2mLC3xziAz59ha9slSBZUDQc3+Wc9YcKPmgoCf8qpqfYywv/X4sjr2WBUCDx/jXueCKXFi01iWVTFPgi6IyblQa1kBJoU/O9vF10e0zWo2kWgpuGWWMIXE4XVaIuSysBLvyPQASWZn8i/vj3cBlXeN31Ii7hWouP+AXwYUjY8+tlZLD7u679UC68aA/S+5e4LWRxQUDjXU3zQk+kBODByPJT9EPwG/pS+RBAjhTHGYiss9dGFN/Bpdr0X2Zo/2C/q1UJjJF39QMx/i4qWidlIZNRby45EFv4aWomwGTdpMVg46xqsck4qi9PVbZ/sKsH1+PF/pxq1xh490mOXQAtokmkqL2/KifXzuWCPUBdyMLVoGcSxnSeNaxIp0TszV3KI2NRN X-Bogosity: Ham, tests=bogofilter, spamicity=0.003057, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Oct 24, 2024 at 11:29=E2=80=AFAM Huang, Ying = wrote: > > Hi, Kairui, > > When reading new swap cluster allocation code. Sorry, I didn't find > time to review your patch at the first place. I found that in > cluster_alloc_swap_entry(), the following code path is possible, > > cluster =3D this_cpu_ptr(si->percpu_cluster); > offset =3D alloc_swap_scan_cluster(); > ... > spin_unlock(&ci->lock); > spin_unlock(&si->lock); > /* migrate to another cpu */ > spin_lock(&ci->lock); > spin_lock(&si->lock); > cluster->next[order] =3D offset; > > That is, the per cpu cluster of a CPU may be changed on another CPU. I > guess that this will not cause some functionality issue. However, this > makes code harder to be reasoned. Is it possible to avoid unlock before > changing per cpu cluster? Or, give up updating per cpu cluster if we > need to unlock. Hi Ying Yes, I've noticed this when working on the new si->lock series. It may cause fragmentation, only if the CPU the current allocation migrates from requested another swap allocation, in such case it may waste one per cpu cluster. No functionality or leak issue, but make things harder to reason indeed, as you mentioned. In the new series I used a local lock to prevent migration, so the cpu is stable during the whole process. However that is not doable unless si->lock is removed from allocation path or there are lock dependency issue. Do we need to fix this separately in the current tree? The fragmentation is overall better with these patches, so I think that is causing any issues. Before these patches per CPU cluster can also be stolen by other CPUs so these were not guaranteed to be accurate. One easy way we can do is set the cluster->next[order] to 0 before scanning for a new cluster to use, and check if it is 0 before setting next[order] again. That may still cause fragmentation as migration will cause extra clusters to be scanned and used, which is hard to avoid without changing too much code (like the things being done in the new si->lock series).