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 1BABBC27C55 for ; Mon, 10 Jun 2024 13:56:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7DE5A6B0085; Mon, 10 Jun 2024 09:56:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 78D9F6B0088; Mon, 10 Jun 2024 09:56:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 656296B0089; Mon, 10 Jun 2024 09:56:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 4AB6F6B0085 for ; Mon, 10 Jun 2024 09:56:15 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id BE7FD410CF for ; Mon, 10 Jun 2024 13:56:14 +0000 (UTC) X-FDA: 82215128268.15.9DC1E25 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by imf17.hostedemail.com (Postfix) with ESMTP id AC75A40012 for ; Mon, 10 Jun 2024 13:56:12 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=B79aVZ5m; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of usamaarif642@gmail.com designates 209.85.218.46 as permitted sender) smtp.mailfrom=usamaarif642@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718027772; 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=aQ41PO/kyUDkz1qf8WP62Viu44Es8AhqI8N2duos8vw=; b=gMZvh5NwO+7XKK+IN/6G4WdVL6NWHUGNP7LTKhkrRFH1yW9PFW7JhYCOEhCpODrbQMd1Rd yQhO+ekQfJm8NSxuocJfjw9zEHcpu2OzjXPmr3zcAxxb1BQa6q1YYsXgzFw5NYr5MbE3wV fji1+PRV2jfwI/i3rxiRB5N7b5otwPM= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=B79aVZ5m; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of usamaarif642@gmail.com designates 209.85.218.46 as permitted sender) smtp.mailfrom=usamaarif642@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718027772; a=rsa-sha256; cv=none; b=RlLTZjg1DEtzNWSHS8whoRp5iYKL/pMAvbyZfRC/B6ER8NxRppn/VbByR5HOi+Fo4SXxUf WdVF+vt4K+fw3sFiMlnNTG3gPeyUJ3bAi75rtXwF/w+xnaeDoszEaIpEEZX+nuJ14o1uQR 5hFwDNn90B4+edBJAvphjrjcgymjIIE= Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a68b41ef3f6so494429866b.1 for ; Mon, 10 Jun 2024 06:56:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718027771; x=1718632571; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=aQ41PO/kyUDkz1qf8WP62Viu44Es8AhqI8N2duos8vw=; b=B79aVZ5mXbnpJ6VAFuyZfuAgsApwsU7Nxp1Xy71TrBryzGvaG/0B3yI22xLM1WoWyj 4xZAHwBXEo/vQImX1dmhxcjjrb96mor+Grz7DxF1oh7chctUTwEBP3ea97lsbGRIn9MT W18Ueei1xoPiIQxXT/LoNm/9So7eQPgsrfb/SX38QhRWRjhVOOK5k//VaMMrqgi4BeGc 5RpHyXHZf0dk2KEg70dE60l0MX3G0cX83yWE2SC0LlnVR0f+wmYapS2VVIPBZv22Iozy VOWrfgFGG0s5t6IMID/UhMY+dfs+9pAgOMnOxD42ojNnRkPbZmrARwq6Q62PWCK4uSXc atrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718027771; x=1718632571; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aQ41PO/kyUDkz1qf8WP62Viu44Es8AhqI8N2duos8vw=; b=R8Oqd5gtkECUN5w9MFOtQzjWTlCgqFtW1EpsTfjQWs10j+JysiNNu+/wgLCa1KVBOs vzojsoRjR1rVdzUKsdslapKbI86WziH8471UyazRR5m9ZCY22/0uc88Qbet9u2gUMBuv 1zwtNAiTRqf4jMzRHugjcjaDcAxGKKBR+OLHsj4svY23xThz6JnHSsM6DizY7oUwBEiO sLZfWqSBhaH+B2EOaIT6PoXtvVak1wA4wyEqu+yn2SmXUz+bTMzYRGcgu9/Nb29VToKM pON22jCKpD5Liwj5g/IvJUIgn8QQjNguVovslGgffujruG6ZgUic2tFYW+J5kPYiPb62 ovQA== X-Forwarded-Encrypted: i=1; AJvYcCX+h4AEm7hyERpT2Sbi37Y8umeBABOV3Hj5f/soQ3lYBS/rpadjck8LuxeehQdlcDu7ar0GqqWABcgKIkgNth/x9wg= X-Gm-Message-State: AOJu0YxGi1T4qXjuFBgpUm++ONrsgN4/qq1RJ/EFOwQDG09VQhF4UDCi 6I7AhmOaZnVK/O3kw4k4rIKRFW4r/5o00ydUBcfg/r862xZqBsTM X-Google-Smtp-Source: AGHT+IHQ/rOiObPfMmeBKsGdRFLQtrCxPbQVsHgRRg9IsrgMkC02S7nOAhNtOQxrNJLrkkDF0o0zng== X-Received: by 2002:a17:906:a888:b0:a6e:5526:e574 with SMTP id a640c23a62f3a-a6e5526f9bemr432213666b.46.1718027770625; Mon, 10 Jun 2024 06:56:10 -0700 (PDT) Received: from ?IPV6:2a03:83e0:1126:4:eb:d0d0:c7fd:c82c? ([2620:10d:c092:500::7:493]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6f0e858d7fsm299097566b.127.2024.06.10.06.56.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Jun 2024 06:56:10 -0700 (PDT) Message-ID: Date: Mon, 10 Jun 2024 14:56:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/2] mm: store zero pages to be swapped out in a bitmap To: Matthew Wilcox Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, david@redhat.com, ying.huang@intel.com, hughd@google.com, yosryahmed@google.com, nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com References: <20240610121820.328876-1-usamaarif642@gmail.com> <20240610121820.328876-2-usamaarif642@gmail.com> Content-Language: en-US From: Usama Arif In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: AC75A40012 X-Stat-Signature: yndgirt6rqcewt4j153mo6zoxkio6zx6 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1718027772-303885 X-HE-Meta: U2FsdGVkX1+qgthGkU47imsrEo8ZDR89WRFfklI8uEigLO85faWcX0BessWvPluQFL4jE/puJtj2DP+lJsaOGOMTt0dSwGVPRBs4i0ihkv0YDaS7UJcI3j5aRJaeHLgvVuZbvBIR9iZrrqmH1sf1woPxrKe1LBxSJQJ728BlkGG4mJHZNdL6W1Sczqb5wkb260dsOHJp/hpVUwBuadHUO1ycW6teBYLFv6YC6HwCdRD4a3QD2oA+asm6kjz5VWx9jbNySfBi6kMDF0cQu9c6xPKERKLuVpjYyxzjp+IcLK94x0P4hMRNXemG9KOjwTijvYbqkUCB2auuj3EKTqbGK9KFHvDGEiXaYU5ZF5UxbovOd2uBmFrMa+UgSGMa0T6uUcCxYSY5/Buv5k38ayWvZCJBgY1Es81QwTdvAhMxmj+nypuxNkSLTPI/EV++xZrGj2XeLWwfJ5Av3BEBsqCQGStTdgnRgKkvfTsx8RmOUpIXuuNV20eYF3x2yHFp9wTF+HZlanvrPO242WEvLlvaoy+ftCL/CexcRGr4Q8/AS3H4pUPyRIUC1OBO/Jnu1ZUK9HZVAJ66uh/iZqNs7FcyLWfsUlXErVNp5GO9T/weVI5q9/53YuvYoRxIHevIaTGx5+hlIxgHzkI47IWP4tfVr4Ndbx51nlzenFUpqch9GLZwdjWOfxB/NlAlm/RSh4UeuNKbyvzIkmMtktPSewP3Kmb4+6oPI2BJaHmMHi+WaZDdYcc872jUjZmfJXhX+UItdiOpeRfLgGxYz+3cColcmjqdHPSGLqyBDDwFf1OJ7J2FvZ48sDS0wbXj/mE5l5xQwpkWwQxRR44T8/cQ40Wf4Q9WpeBsj2jwWkxrysIRyF1fvgcK1YN2uphCH9gUDTXbAqo/d3tPZBqGoGBMxgIZa5q05eWDA2s+yAN9xcURNKrJFdTQqms2Syg2NL25IbM9XtlcHBTqvA538K9n6l4 QmLA5Bj9 xVjMPwKm9uM/1Vs42KP1byQppzVFRQ8+NN16LPbbofPOr1ONDfnHKAZfKul9vz/oVz7OWulutMR+MoM0UiRoB3qnUGwNT9bEV6fM05hq+MjFpkZkjf2YxyvJmcM1E3KI37jwbcG7k/sEuqWgfWrTt6Onik6fa6zqE4wPmd+YxI3TObeTAMf51/cyZBwvS7nf7MzN3McR2D6zQD/9Lt/b49cLJV43HLBnWgZ6CeJY8pNzK3VY1+tIFazalKAYeaB4wfLrc5F+NTEc680XDbXK86kRNvqFyz1TFndL71ZNocZQ37qinOhkIpc+mTUlLRVKGutpByPvp9wljDgsDeVx7z4BGq2W50YKFya9Bsf0kOgAkfdMu6v4pkPm74FmhmrIamYE7NU3cNeYOBsLTsXlp5FAmqj75iCxalGDptX8gf/lKDDYZ4WlbghTQt9rhVRWKL7E+I+0eKzNuYmhV35+WKwVIGB3aQ/WWnIWWcn54Mvcm8oy7Rp3icdfJU42sNiuMfsubtMy9c31+MMlpihAoO4l//tK2NcMN29+yDSNO+FD7FoyhHkq9B/BSSNW8H3AEMBqSgxzAUu/9i34LuwzhVB2j4N6hlVP7s7SFlySgAEWmIDCw0GYsyfN5iA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.002151, 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 10/06/2024 14:07, Matthew Wilcox wrote: > On Mon, Jun 10, 2024 at 01:15:59PM +0100, Usama Arif wrote: >> + if (is_folio_zero_filled(folio)) { >> + swap_zeromap_folio_set(folio); >> + folio_start_writeback(folio); >> + folio_unlock(folio); >> + folio_end_writeback(folio); > What's the point? As far as I can see, the only thing this is going to > do is spend a lot of time messing with various counters only to end up > with incrementing NR_WRITTEN, which is wrong because you didn't actually > write it. > I am guessing what you are suggesting is just do this?     if (is_folio_zero_filled(folio)) {         swap_zeromap_folio_set(folio);         folio_unlock(folio);         return 0;     } This is what I did initially while developing this, but when I started looking into why zswap_store does  folio_start_writeback, folio_unlock, folio_end_writeback I found: https://elixir.bootlin.com/linux/v6.9.3/source/Documentation/filesystems/locking.rst#L336 "If no I/O is submitted, the filesystem must run end_page_writeback() against the page before returning from writepage." and https://elixir.bootlin.com/linux/v6.9.3/source/Documentation/filesystems/locking.rst#L349 "Note, failure to run either redirty_page_for_writepage() or the combination of set_page_writeback()/end_page_writeback() on a page submitted to writepage will leave the page itself marked clean but it will be tagged as dirty in the radix tree.  This incoherency can lead to all sorts of hard-to-debug problems in the filesystem like having dirty inodes at umount and losing written data. " If we have zswap enabled, the zero filled pages (infact any page that is compressed), will be saved in zswap_entry and NR_WRITTEN will be wrongly incremented. So the behaviour for NR_WRITTEN does not change in this patch when encountering zero pages with zswap enabled (even if its wrong). This patch just extracts the optimization out from zswap [1] to swap, so that it always runs. In order to fix NR_WRITTEN accounting for zswap, this patch series and any other cases where no I/O is submitted but end_page_writeback is called before returning to writepage, maybe we could add an argument to __folio_end_writeback like below? There are a lot of calls to folio_end_writeback and the behaviour of zeropage with regards to NR_WRITTEN doesnt change with or without this patchseries with zswap enabled, so maybe we could keep this independent of this series? I would be happy to submit this as separate patch if it makes sense. diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 81b2e4128d26..415037f511c2 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -3042,7 +3042,7 @@ static void wb_inode_writeback_end(struct bdi_writeback *wb)         spin_unlock_irqrestore(&wb->work_lock, flags);  } -bool __folio_end_writeback(struct folio *folio) +bool __folio_end_writeback(struct folio *folio, bool nr_written_increment)  {         long nr = folio_nr_pages(folio);         struct address_space *mapping = folio_mapping(folio); @@ -3078,7 +3078,8 @@ bool __folio_end_writeback(struct folio *folio)         lruvec_stat_mod_folio(folio, NR_WRITEBACK, -nr);         zone_stat_mod_folio(folio, NR_ZONE_WRITE_PENDING, -nr); -       node_stat_mod_folio(folio, NR_WRITTEN, nr); +       if (nr_written_increment) +               node_stat_mod_folio(folio, NR_WRITTEN, nr);         folio_memcg_unlock(folio);         return ret; [1] https://lore.kernel.org/all/20171018104832epcms5p1b2232e2236258de3d03d1344dde9fce0@epcms5p1/