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 05489C27C4F for ; Thu, 13 Jun 2024 19:27:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 860F26B0082; Thu, 13 Jun 2024 15:27:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7E9786B0088; Thu, 13 Jun 2024 15:27:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 662D06B00CC; Thu, 13 Jun 2024 15:27:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 4244E6B0082 for ; Thu, 13 Jun 2024 15:27:24 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E4D79140322 for ; Thu, 13 Jun 2024 19:27:23 +0000 (UTC) X-FDA: 82226849166.27.CBC5B72 Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by imf07.hostedemail.com (Postfix) with ESMTP id 02B4140015 for ; Thu, 13 Jun 2024 19:27:21 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="N/rd2J+W"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718306840; 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=t9orlxNKn/9JVZXU5e2lmdZ8oEkBOmdjYH2JldRLXf8=; b=6oGMwcCHOIS6DGyXq6OanhrinvTV89zJSxZMyAXTAXSPhT9iuaR5XX1CR/bjomVDyZN+03 Y7CwDkce+jzU3mssYQaUDALu4ceGSh6IMDq5SUNM5ZUCz12R4VWiUFCaMc/ZekJ3GWjlPn fn6ls3jG5TyuRgHZZsv2kkHfJBKktuI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718306840; a=rsa-sha256; cv=none; b=H9iev1D3XSCDKQaj0o9MCzF2HOnzX2/kKb0NMicpHY/KTvAQbYgurnup/MVmIQJZ2M+s5c 9y9ur883s+5JnvM1KNEGH3L0u5xhSKGR+e6JntFWHt17kPgKmj2s0Ky7lpy/uRqxoBeJnn UEytcglSzOb3ZrhzeHsemkcOKTqb4Z8= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="N/rd2J+W"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=yosryahmed@google.com Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-52c525257feso1829172e87.1 for ; Thu, 13 Jun 2024 12:27:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718306840; x=1718911640; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=t9orlxNKn/9JVZXU5e2lmdZ8oEkBOmdjYH2JldRLXf8=; b=N/rd2J+WZcDaH8OQ1QfCZumwVPYgGiNHP1QcmI4JaX7f9LS8k0EGLOLd/u8gUTjXfv Xy00w+OxJpbzxlr9go8gsO0pJWEIs2b6fIbXrskaXOXqqVU060tBuR/KWA1cvy8PHVVg eGE15rlj5gIBzGSuj8txsbWYaJwHhCjYNXpcbEpDD+mR/QghUvE1sojJIr8P4v2x2mne NtwkzQe/F+T0myzJA6yrCgxop00rKtxW2iK6b7xytZO9P1So1xib+yyzDiJCPxYIH75L DAITLLsFUEbEbdbewWhDYnInCl6N2ThmXf8huOuSICL+cA/XVuasGyzf+iuzn+ALLLDj wtjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718306840; x=1718911640; h=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=t9orlxNKn/9JVZXU5e2lmdZ8oEkBOmdjYH2JldRLXf8=; b=NhlLSqC3EJrRtdUKjcSDYzn7PjwkHYAf7S+zIQFVh84TmcAdqUNHAzkkhvKNuA+JaC sLlxV5sQJOf/TVrH3GzY8REzOOxlSZiDdIZ4PACNwMNstHBseWSloRUFFudloPCphR0i ji9SRKPJc+U7MeCFeYKVo9OtHoyjSx9Guj6wlwzWqENDwXaHgW4tvKHscIStsPLrXu+s esV9sczsvuV4erCGncWvicoylhiujy2uUd5UlBaP3RTsoVwi7XB6a1hmPUBwI6OGj91j +yyprqx4HdBjBqPYoBTgJksISmGVakcWN44IZQsvVMJ5LH+taZzQEzen0lwxuNFs7GFV XN7Q== X-Forwarded-Encrypted: i=1; AJvYcCXYv1Zi9K+D5DIC9HZsIUGnNaO3dD94Q6+e6p7RDJg/cGZaLoOdd1gMP2khoalOv7J1dYhK4mSNH+TEQytg8lKmdok= X-Gm-Message-State: AOJu0YyCxq3eDxdzV9r0zukd04al3CpZ55jsH/KVUt6Y5EEs2XIEXKVk I1AYhfHRPEBZXTUPDFpzSxe6CwROWDVtS5DLviPb04sEgwlYx5tKGeDsRVb133FnqaFxV9IZ6HJ WFart5cm4z2KGhx/bU50pa66w3IjCXA2OOxUM X-Google-Smtp-Source: AGHT+IGzN6TmSH1p6vXFvV8Km9FazTBEXlJalb8xYy/AJ+qb/Mo1wKanhXT9P+WqKg+UjLbGK6D2OeLnBXEtlJDXYHQ= X-Received: by 2002:a05:6512:130e:b0:52c:8f4a:9200 with SMTP id 2adb3069b0e04-52ca6e9897emr543765e87.65.1718306839832; Thu, 13 Jun 2024 12:27:19 -0700 (PDT) MIME-Version: 1.0 References: <20240612124750.2220726-1-usamaarif642@gmail.com> <20240612124750.2220726-2-usamaarif642@gmail.com> <85804484-9973-41a1-a05d-000833285f39@gmail.com> <0572d8b1-3b17-45a8-bf75-f66e19216d38@gmail.com> In-Reply-To: <0572d8b1-3b17-45a8-bf75-f66e19216d38@gmail.com> From: Yosry Ahmed Date: Thu, 13 Jun 2024 12:26:42 -0700 Message-ID: Subject: Re: [PATCH v4 1/2] mm: store zero pages to be swapped out in a bitmap To: Usama Arif Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, shakeel.butt@linux.dev, david@redhat.com, ying.huang@intel.com, hughd@google.com, willy@infradead.org, nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 02B4140015 X-Stat-Signature: 6iej1crb1rcsqhjqujppn785o879cdsp X-Rspam-User: X-HE-Tag: 1718306841-665601 X-HE-Meta: U2FsdGVkX1/HCckq3Fwrd28FrpVCG/wgRPcBb28GRYOxvXZF+huZHm242glNDij1O4A2RWGfKbfaLXXwC88sShfhrt3dzNF96AV5TbHeyd5QgQiEe6sVRUYHEJ2/g7Vm/W6UNQXhXFhle5aFTwl3DxKDjwxiqmrSuBomwr+I1LgMBYgK91xty1VH8XuOb5rVa05QGxzr5TZEQgKDbD+wam9YUN3Qn6lR4Zh/7l06Bgal5arcYX1scSd1jXCHlBJIbGfQaZlQO7m/ucjIZ4km/RBHApS59PZeZ1pggxEzeiV45lv9qqCEUb/CjbFC/0DVZyv1nEuVHysK8M83mtlCMiv6zUDmo9PTAcbpj5xx4tGUMrIi/YlucMY/3p8RFP75yEukZ6JQRvfEnkgRTRMDVNH47kLKR0mrCCDc1mXZoW+gTli7qxTC2nb+XXeLqKjTNZwYmt7zDaCNo2s1Z3IfQZ6K6ksnSaA/1x9j/JAnakGWff6gMyW0iziUddvARINqiX5tCfnh9mZVyTpgVf/o/X25WX8nnU1OJoQEL34dTkfE8K6bAGr7D17gqwMBJO/q3nXlm1WIO8Zoh6y/v8wx0OJarma2LKircoLapT0pCycljcyFhYLcN/xRp+omAOZ2xBxG0/jCF8NUrlZRYwrCK3nEoaclhD926QiNX4Urrv8w8WqHppBfiHiXcuYK4hkcIX4hvDgopPlJ57Fe7K/HcHnzK1cBib1nqaV2Ui01xToAFLH+pugP+l9Qs4gWPSc+HmcfhlkbTVtu9dOC2UeWRjOfDrAOJLaMUmQng+27tvJoJaaCpFCKC/+F1rQO2a9k6SeFJVw7U0yaVcnUPLK/j+YHY8an80VAZQ2g87FTCgpmeRQ+0TTCv+7VoeSpvugO6rDzEJc3dWcU5b7bEdnJ2dIue1xtBFd1QyoVukFQ1JV0ECcnFNjJ9+N2No7O+MeBmopEhC9qEL3LNUc+eX2 aaC4zp3Z 1v0b5aMYl4DRZRTtpj4qmxZ9/satzLTpMqY22VNLTc/bcivELYo11zO55hIWpXT9TCPxI0p4Sasj3wUrEGFCeQlU6Zjcx+WdCxYT7sbxzv8CcK6XI2Sq5VZF5yxOtgIex/pO8MN1OWhB9pei2flwE8iC8/sEiMNJOsfU+yCztYnDMklc= 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: List-Subscribe: List-Unsubscribe: [..] > >>>> @@ -498,6 +507,8 @@ static void swap_do_scheduled_discard(struct swap_info_struct *si) > >>>> __free_cluster(si, idx); > >>>> memset(si->swap_map + idx * SWAPFILE_CLUSTER, > >>>> 0, SWAPFILE_CLUSTER); > >>>> + for (i = 0; i < SWAPFILE_CLUSTER; i++) > >>>> + clear_bit(idx * SWAPFILE_CLUSTER + i, si->zeromap); > >>> Same here. I didn't look into the specific code paths, but shouldn't the > >>> cluster be unused (and hence its zeromap bits already cleared?). > >>> > >> I think this one is needed (or atleast very good to have). There are 2 > >> paths: > >> > >> 1) swap_cluster_schedule_discard (clears zeromap) -> swap_discard_work > >> -> swap_do_scheduled_discard (clears zeromap) > >> > >> Path 1 doesnt need it as swap_cluster_schedule_discard already clears it. > >> > >> 2) scan_swap_map_slots -> scan_swap_map_try_ssd_cluster -> > >> swap_do_scheduled_discard (clears zeromap) > >> > >> Path 2 might need it as zeromap isnt cleared earlier I believe > >> (eventhough I think it might already be 0). > > Aren't the clusters in the discard list free by definition? It seems > > like we add a cluster there from swap_cluster_schedule_discard(), > > which we establish above that it gets called on a free cluster, right? > > You mean for path 2? Its not from swap_cluster_schedule_discard. The > whole call path is > > get_swap_pages -> scan_swap_map_slots -> scan_swap_map_try_ssd_cluster > -> swap_do_scheduled_discard. Nowhere up until swap_do_scheduled_discard > was the zeromap cleared, which is why I think we should add it here. swap_do_scheduled_discard() iterates over clusters from si->discard_clusters. Clusters are added to that list from swap_cluster_schedule_discard(). IOW, swap_cluster_schedule_discard() schedules freed clusters to be discarded, and swap_do_scheduled_discard() later does the actual discarding, whether it's through si->discard_work scheduled by swap_cluster_schedule_discard(), or when looking for a free cluster through scan_swap_map_try_ssd_cluster(). Did I miss anything?