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 84765CD1283 for ; Fri, 29 Mar 2024 04:27:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E774E6B0088; Fri, 29 Mar 2024 00:27:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E27BC6B0089; Fri, 29 Mar 2024 00:27:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CC7A46B008A; Fri, 29 Mar 2024 00:27:58 -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 AE8656B0088 for ; Fri, 29 Mar 2024 00:27:58 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 68D35121132 for ; Fri, 29 Mar 2024 04:27:58 +0000 (UTC) X-FDA: 81948793836.12.249B0A6 Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by imf07.hostedemail.com (Postfix) with ESMTP id A11B740005 for ; Fri, 29 Mar 2024 04:27:56 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=4ZbtlFdf; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711686476; 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=c/vAyJ4a5RQBFlL1Mz9qHhkQoRCF3uS+NkBJETvCGyY=; b=NUA4Dy3QikjDz9T4KBLQOgo6hoZuW84Q5KDksSxwtNbVB335mp96/q9qLFdrKiHmNvFPli S804NsAirlqrrhka7EfzORdFk7FuDFrNxlhsnc+8srtWxX0/gelZaoRdtv+gfcPKhrM0f+ FfMq3VLs9i+sjdgQcL8qC6aHI1ghHpU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711686476; a=rsa-sha256; cv=none; b=GugO9jPvDLzOJvzg5pe0wzlug5LDknNyOa3WsIrOCj8DGT+0FFjLqKFu+p8iqQ+Py6DBtv 2QNm3+jMwL/xrjT2BCurcQ8+juGeNUdqCOIlWr4xP8elaQMwW9BL+Zi2V4gsjtv8NXnN+d nbJwRrMnW0YpAKBoAw8qVkVsmEtedUk= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=4ZbtlFdf; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-lf1-f43.google.com with SMTP id 2adb3069b0e04-515cd2ae7aeso553444e87.0 for ; Thu, 28 Mar 2024 21:27:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1711686475; x=1712291275; 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=c/vAyJ4a5RQBFlL1Mz9qHhkQoRCF3uS+NkBJETvCGyY=; b=4ZbtlFdfplkkTVLmYELV0CyDVraX/gyMzYqiKWajR7zpgBoKMuW5mSyD2nWSozpomG 2jX8j2NGWWtEZUaEtEjEEREAd5ggKwpU1MQWs4PHYq2BPqCmtfrq2ysddfuyoLHJ/22P brDDmyRWnC56VhMHp/3LDqMpZa5YPCQMT0JFGCNWJE0ISmyYeSoN5+/W0wGnvZXhhtNZ aSZMvjaHZYcIOPAlNPRjtP/h1p+Qhpizw6ceEEd3ihW1ookZqfLnKWzJxoda0UA2seRb THoLiU065Kjl+bidcOK8khG4VYNvDfdVwfzu6EEi2r03vf7+MXYOgz7juC79puC0oAOV br4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711686475; x=1712291275; 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=c/vAyJ4a5RQBFlL1Mz9qHhkQoRCF3uS+NkBJETvCGyY=; b=g0Dl3FT5oBnR53W6mAalABp3CYCN/kSby+G9YlcEjzcnreynxcYe8KDCaDk8h4tBRi XlOBNS8NGtseCzwDun/tyYnn/SfcfJjZ5YoGXZUD9OHPQADHDbieKpoLbZ2lziqyIpk8 B9+3ppasPvS6bXQZBFF8cgte/RvBJQlfZvq4nZ0OWMMVv711MyNxmnGiciI37OHPzS4B EFgbgFz3Pic6OJNd5as+opOF8BOOVb4Jj7QuwQJIau1D/n+gIQdLDkwvN7y0caoFLzZS 1zPhGwqn9NoqhXKe3XyLoOrOC8IjT+hdkO1KVKGXdxpqHf1MBaeNkuVP+Aws0wF9Y3RL arQw== X-Forwarded-Encrypted: i=1; AJvYcCUHUI0MAH42b3vPEHg6uvmw9bNXlhffjP9e6FmaerzY9ZM0h0oyClW87p8iB3lWKKp2D0rVhp58ahTOKmUkvwA9Dng= X-Gm-Message-State: AOJu0YzHXjcLhP5izpPPqRwXK85Ro2YhYHQSuFnydcLNrKXBrBYb5syN F7ehpZ0xr7yI8w/cxLDjTwOY0kYIfFFYn5eLgmAKpmPYqPnWYjwQ6OZO0M4HStBOZO9+RPWanMl miszd+Ge4JZ2DGO8n3ufi23OsQpOpZcEz4D+3 X-Google-Smtp-Source: AGHT+IGJk5o1bduZOx1XkmIoLSUMm6ZQnfDKEkl2dt0ysbqUb2ufvf4hhNYO99cHR06Sr1kn5VgzlsS+iCHEX9AHYZA= X-Received: by 2002:a05:6512:48c6:b0:515:cedb:a518 with SMTP id er6-20020a05651248c600b00515cedba518mr526032lfb.16.1711686474537; Thu, 28 Mar 2024 21:27:54 -0700 (PDT) MIME-Version: 1.0 References: <20240325235018.2028408-1-yosryahmed@google.com> <20240325235018.2028408-7-yosryahmed@google.com> <20240328193149.GF7597@cmpxchg.org> <20240328210709.GH7597@cmpxchg.org> In-Reply-To: From: Yosry Ahmed Date: Thu, 28 Mar 2024 21:27:17 -0700 Message-ID: Subject: Re: [RFC PATCH 6/9] mm: zswap: drop support for non-zero same-filled pages handling To: Nhat Pham Cc: Johannes Weiner , Andrew Morton , Chengming Zhou , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 1318bgiioo9emtibw5pb9dzu8y6a1rog X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: A11B740005 X-Rspam-User: X-HE-Tag: 1711686476-771945 X-HE-Meta: U2FsdGVkX1/FwxAXQVePF5vACdIFpvTtw8DbC0bAIloMjHsuTRKAiLllro6Uso1WPe1O/rrnAykJ2RiXATDCFWxSPGV2zlPfwhjKoY/2iXZxlYBQU2p838K2T+tGetrXRbi5S2wZPZ0MgS2U23DMTd/eHqXvaI+83e2JMpviHY/9bt6RvGb2Sr35IJznpSM97PrBu34zwaBwTrd8cbGKo7V05ODsGg1C5xbS7J8x+8EBbI3j0d8Gr0TUc9cmPS7mFtwK3NBS44MqXx4ZiHxSvtihGDBmTy4DOTNybHtRwweQF4He73SVCzkKmQcoB3m335XXH1FZnMzvURlcvh6cVMano7t4K/GTToadbpIjTD28PLWQmTBZ+oOcRXYgctar6NhpZgw/oozw61sTYMw8OpCvUJkftQtBvRdnsE7RLaYE2XZWNwGvre3S2lEuU0z49k2fuSMBpGELWUA418mTaHwLahF6Q6bt38XexVjJ9n7Hvcd9pTzzzqOw2ETIxLSLW+fRmGhnpGQtbUDzvF0IOY7GZlH/QrAz0qiOSne+1xxSC8mpTNRk19ZEH7kyL0q/yHil3cLKl7bdmhXLnXhrhXkwMDQnFo3WYFnGe7+0OwpYvsaOrbF/e/+Vljr95fUjwe+0dc/hCwAOhoJQC4tKN5ObCturHWNXAS7Sgt4NRqrtr2NmrFdSVrQ0FGr4QEw6hY8ZsJhkFS01EUs4jA7kYvHAyt9j7KhU42FUEaLjnMFpz2BBa04Elxs4BDEOHxdG0bb1dJjS3ZaoXZdX251Manc2xkR1S3JDnxVvwHDuQU6lalgJuT6oaa+wDGEQLW2fjNZYlxSWtWJthOGI5oXVnwOD4Y6AEYcDSwyn0y1GnY2FNX6B+nj7a2FoVrlxbVLVLJQAzlBq4Y+fectQRmOq6vZXRiLy4qC2tXYsLVWMk9wB1LKRye/uemNwtflPKnu++HqE64ALcT4wyA1FSU8 BfQ== 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: On Thu, Mar 28, 2024 at 7:05=E2=80=AFPM Yosry Ahmed = wrote: > > On Thu, Mar 28, 2024 at 4:19=E2=80=AFPM Nhat Pham wro= te: > > > > On Thu, Mar 28, 2024 at 2:07=E2=80=AFPM Johannes Weiner wrote: > > > > > > On Thu, Mar 28, 2024 at 01:23:42PM -0700, Yosry Ahmed wrote: > > > > On Thu, Mar 28, 2024 at 12:31=E2=80=AFPM Johannes Weiner wrote: > > > > > > > > > > On Mon, Mar 25, 2024 at 11:50:14PM +0000, Yosry Ahmed wrote: > > > > > > The current same-filled pages handling supports pages filled wi= th any > > > > > > repeated word-sized pattern. However, in practice, most of thes= e should > > > > > > be zero pages anyway. Other patterns should be nearly as common= . > > > > > > > > > > > > Drop the support for non-zero same-filled pages, but keep the n= ames of > > > > > > knobs exposed to userspace as "same_filled", which isn't entire= ly > > > > > > inaccurate. > > > > > > > > > > > > This yields some nice code simplification and enables a followi= ng patch > > > > > > that eliminates the need to allocate struct zswap_entry for tho= se pages > > > > > > completely. > > > > > > > > > > > > There is also a very small performance improvement observed ove= r 50 runs > > > > > > of kernel build test (kernbench) comparing the mean build time = on a > > > > > > skylake machine when building the kernel in a cgroup v1 contain= er with a > > > > > > 3G limit: > > > > > > > > > > > > base patched % diff > > > > > > real 70.167 69.915 -0.359% > > > > > > user 2953.068 2956.147 +0.104% > > > > > > sys 2612.811 2594.718 -0.692% > > > > > > > > > > > > This probably comes from more optimized operations like memchr_= inv() and > > > > > > clear_highpage(). Note that the percentage of zero-filled pages= during > > > > > > this test was only around 1.5% on average, and was not affected= by this > > > > > > patch. Practical workloads could have a larger proportion of su= ch pages > > > > > > (e.g. Johannes observed around 10% [1]), so the performance imp= rovement > > > > > > should be larger. > > > > > > > > > > > > [1]https://lore.kernel.org/linux-mm/20240320210716.GH294822@cmp= xchg.org/ > > > > > > > > > > > > Signed-off-by: Yosry Ahmed > > > > > > > > > > This is an interesting direction to pursue, but I actually thinkg= it > > > > > doesn't go far enough. Either way, I think it needs more data. > > > > > > > > > > 1) How frequent are non-zero-same-filled pages? Difficult to > > > > > generalize, but if you could gather some from your fleet, that > > > > > would be useful. If you can devise a portable strategy, I'd al= so be > > > > > more than happy to gather this on ours (although I think you h= ave > > > > > more widespread zswap use, whereas we have more disk swap.) > > > > > > > > I am trying to collect the data, but there are.. hurdles. It would > > > > take some time, so I was hoping the data could be collected elsewhe= re > > > > if possible. > > > > > > > > The idea I had was to hook a BPF program to the entry of > > > > zswap_fill_page() and create a histogram of the "value" argument. W= e > > > > would get more coverage by hooking it to the return of > > > > zswap_is_page_same_filled() and only updating the histogram if the > > > > return value is true, as it includes pages in zswap that haven't be= en > > > > swapped in. > > > > > > > > However, with zswap_is_page_same_filled() the BPF program will run = in > > > > all zswap stores, whereas for zswap_fill_page() it will only run wh= en > > > > needed. Not sure if this makes a practical difference tbh. > > > > > > > > > > > > > > 2) The fact that we're doing any of this pattern analysis in zswa= p at > > > > > all strikes me as a bit misguided. Being efficient about repet= itive > > > > > patterns is squarely in the domain of a compression algorithm.= Do > > > > > we not trust e.g. zstd to handle this properly? > > > > > > > > I thought about this briefly, but I didn't follow through. I could = try > > > > to collect some data by swapping out different patterns and observi= ng > > > > how different compression algorithms react. That would be interesti= ng > > > > for sure. > > > > > > > > > > > > > > I'm guessing this goes back to inefficient packing from someth= ing > > > > > like zbud, which would waste half a page on one repeating byte= . > > > > > > > > > > But zsmalloc can do 32 byte objects. It's also a batching slab > > > > > allocator, where storing a series of small, same-sized objects= is > > > > > quite fast. > > > > > > > > > > Add to that the additional branches, the additional kmap, the = extra > > > > > scanning of every single page for patterns - all in the fast p= ath > > > > > of zswap, when we already know that the vast majority of incom= ing > > > > > pages will need to be properly compressed anyway. > > > > > > > > > > Maybe it's time to get rid of the special handling entirely? > > > > > > > > We would still be wasting some memory (~96 bytes between zswap_entr= y > > > > and zsmalloc object), and wasting cycling allocating them. This cou= ld > > > > be made up for by cycles saved by removing the handling. We will be > > > > saving some branches for sure. I am not worried about kmap as I thi= nk > > > > it's a noop in most cases. > > > > > > Yes, true. > > > > > > > I am interested to see how much we could save by removing scanning = for > > > > patterns. We may not save much if we abort after reading a few word= s > > > > in most cases, but I guess we could also be scanning a considerable > > > > amount before aborting. On the other hand, we would be reading the > > > > page contents into cache anyway for compression, so maybe it doesn'= t > > > > really matter? > > > > > > > > I will try to collect some data about this. I will start by trying = to > > > > find out how the compression algorithms handle same-filled pages. I= f > > > > they can compress it efficiently, then I will try to get more data = on > > > > the tradeoff from removing the handling. > > > > > > I do wonder if this could be overthinking it, too. > > > > > > Double checking the numbers on our fleet, a 96 additional bytes for > > > each same-filled entry would result in a > > > > > > 1) p50 waste of 0.008% of total memory, and a > > > > > > 2) p99 waste of 0.06% of total memory. > > Right. Assuming the compressors do not surprise us and store > same-filled pages in an absurd way, it's not worth it in terms of > memory savings. > > > > > > > And this is without us having even thought about trying to make > > > zsmalloc more efficient for this particular usecase - which might be > > > the better point of attack, if we think it's actually worth it. > > > > > > So my take is that unless removing it would be outright horrible from > > > a %sys POV (which seems pretty unlikely), IMO it would be fine to jus= t > > > delete it entirely with a "not worth the maintenance cost" argument. > > > > > > If you turn the argument around, and somebody would submit the code a= s > > > it is today, with the numbers being what they are above, I'm not sure > > > we would even accept it! > > > > The context guy is here :) > > > > Not arguing for one way or another, but I did find the original patch > > that introduced same filled page handling: > > > > https://github.com/torvalds/linux/commit/a85f878b443f8d2b91ba76f09da21a= c0af22e07f > > > > https://lore.kernel.org/all/20171018104832epcms5p1b2232e2236258de3d03d1= 344dde9fce0@epcms5p1/T/#u > > Thanks for digging this up. I don't know why I didn't start there :) > > Following in your footsteps, and given that zram has the same feature, > I found the patch that added support for non-zero same-filled pages in > zram: > https://lore.kernel.org/all/1483692145-75357-1-git-send-email-zhouxianron= g@huawei.com/#t > > Both of them confirm that most same-filled pages are zero pages, but > they show a more significant portion of same-filled pages being > non-zero (17% to 40%). I suspect this will be less in data centers > compared to consumer apps. > > The zswap patch also reports significant performance improvements from > the same-filled handling, but this is with 17-22% same-filled pages. > Johannes mentioned around 10% in your data centers, so the performance > improvement would be less. In the kernel build tests I ran with only > around 1.5% same-filled pages I observed 1.4% improvements just by > optimizing them (only zero-filled, skipping allocations). > > So I think removing the same-filled pages handling completely may be > too aggressive, because it doesn't only affect the memory efficiency, > but also cycles spent when handling those pages. Just avoiding going > through the allocator and compressor has to account for something :) Here is another data point. I tried removing the same-filled handling code completely with the diff Johannes sent upthread. I saw 1.3% improvement in the kernel build test, very similar to the improvement from this patch series. _However_, the kernel build test only produces ~1.5% zero-filled pages in my runs. More realistic workloads have significantly higher percentages as demonstrated upthread. In other words, the kernel build test (at least in my runs) seems to be the worst case scenario for same-filled/zero-filled pages. Since the improvement from removing same-filled handling is quite small in this case, I suspect there will be no improvement, but possibly a regression, on real workloads. As the zero-filled pages ratio increases: - The performance with this series will improve. - The performance with removing same-filled handling completely will become worse.