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 AED9AC4332F for ; Fri, 18 Nov 2022 05:14:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BB5606B0071; Fri, 18 Nov 2022 00:14:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B65356B0072; Fri, 18 Nov 2022 00:14:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A05E68E0001; Fri, 18 Nov 2022 00:14:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 8F0D66B0071 for ; Fri, 18 Nov 2022 00:14:52 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 577AD1A07BD for ; Fri, 18 Nov 2022 05:14:52 +0000 (UTC) X-FDA: 80145398424.07.0576ACF Received: from mail-qv1-f48.google.com (mail-qv1-f48.google.com [209.85.219.48]) by imf29.hostedemail.com (Postfix) with ESMTP id C3DA6120011 for ; Fri, 18 Nov 2022 05:14:51 +0000 (UTC) Received: by mail-qv1-f48.google.com with SMTP id h10so2691312qvq.7 for ; Thu, 17 Nov 2022 21:14:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=cps+dr7Masaue6SEA1cOwam8E00KKDa11kpmkte+dvw=; b=CVfaYOTLnhq+C1vzAVjVijsMl5TkuAn8KHJRqRsjt9JShmfez1AlLLMt/5bxfJ4NFa FZUekTHQ5t/SwrXgj+ae4j54drfL9GA7JgsBIDd4TxgIoVtC/smQjZlIfksaBDwR5XoA bLQeoWjOx7Wh+6Gibnd3UwBu7MfyKUqICNRlsKVed6556jSzEE7CahaP/0V3DZBkoE+k rZnAvQl4vw0cWozhN/zS/kyCMOrQXm61KSUzI+mq/LpY8SDHCtOfrBpBbVF3InN1jjTi y8gMI0VeyAnucaiLpWTaBh0P31E9boMegd01QQCRpWQNQ6owUWz9kv5e8gSPHwdpcxoL aBkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=cps+dr7Masaue6SEA1cOwam8E00KKDa11kpmkte+dvw=; b=WEVWIA+INIfB/3vnSRf5AuILPfYEZo3khyv1FnXPY0oq9lyFE6Rl7rzDmMgZTUPPOz sFjUp/tvAljzqtHk2rzrq1tOh1ii5cY0SqyTIWI2uwcJWzZmbV8nPSqt80aazQKFmnK0 UnG5D3YL9Ss8fRWdcYlRGTeyId1K3fqM8LxdgT13krCv3FXaZVEkyDinjatVTUo0HIX2 dtUXhwZ9pKAIyCGuQOGwsX2TAny/COV2CsE1la+Tc5VHCIu3xfsHLfEp8YuK7BV811t8 g6a1qChyo6g8PeHBsc1N2KBjsdspkWispbrSI4mxRcMZwct9SN2ZWyOt4Llh3k+bFEMc QMKg== X-Gm-Message-State: ANoB5pnz431IxrCHkuhzhYV1RCQg8AGXNZAq+LWa+OQGN6RoqYqVEy+c wM4PEvEhCdLUp7jrmjN3V4DNNg== X-Google-Smtp-Source: AA0mqf6dtwnzkegvPHBy8V3axztXGg1+iQT1gq4xPwbxXY9/sUveZngKtRWxiehoIlFNdzcwDilpYw== X-Received: by 2002:a05:6214:162c:b0:4c6:92da:3c21 with SMTP id e12-20020a056214162c00b004c692da3c21mr1788161qvw.33.1668748490966; Thu, 17 Nov 2022 21:14:50 -0800 (PST) Received: from localhost ([2620:10d:c091:480::1:bc4]) by smtp.gmail.com with ESMTPSA id cn6-20020a05622a248600b003a4cda52c95sm1493120qtb.63.2022.11.17.21.14.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Nov 2022 21:14:50 -0800 (PST) Date: Fri, 18 Nov 2022 00:15:13 -0500 From: Johannes Weiner To: Minchan Kim Cc: Nhat Pham , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, ngupta@vflare.org, senozhatsky@chromium.org, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com Subject: Re: [PATCH v4 4/5] zsmalloc: Add ops fields to zs_pool to store evict handlers Message-ID: References: <20221117163839.230900-1-nphamcs@gmail.com> <20221117163839.230900-5-nphamcs@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668748492; a=rsa-sha256; cv=none; b=LB3Ry6AapJWPYqjZj5gzRXb3gUUofV325a3TXxwDPUT/Bnk6WKtLLhB16g5oKQo4AvchNh BRCBgVcp/tH5gLYBMCtbuv7CtOrVOHlQhHta44xhQ/BGpxsYPHZmMWv9cOwzajRg8dg7xd VKJoVk/sly9Mrjo0nshQJvxYVJ96+zE= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=CVfaYOTL; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf29.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.48 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668748492; 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=cps+dr7Masaue6SEA1cOwam8E00KKDa11kpmkte+dvw=; b=0XpmZeQS9/snYTzEs3pKWmCIb8LPsFGsVAKL4NIUyZeKIiXLyDBaPIoHYIynwZBuAK9U3i Ud7DWK/Pc/13Yo0FxqLthve1meAgiED1qLdhPvsTdWIZJ8v56Rs6Ijhr+xDo8372npy3YD gxSLwa0okBo/53wdd5almK6Xt/PfLEM= X-Stat-Signature: wcaxxsucwi1hdzirdw8hdtqibsjf4x5b X-Rspamd-Queue-Id: C3DA6120011 X-Rspam-User: Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=CVfaYOTL; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf29.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.48 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org X-Rspamd-Server: rspam09 X-HE-Tag: 1668748491-402563 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: On Thu, Nov 17, 2022 at 02:25:52PM -0800, Minchan Kim wrote: > On Thu, Nov 17, 2022 at 08:38:38AM -0800, Nhat Pham wrote: > > This adds fields to zs_pool to store evict handlers for writeback, > > analogous to the zbud allocator. > > > > Signed-off-by: Nhat Pham > > --- > > mm/zsmalloc.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > > index 2557b55ec767..776d0e15a401 100644 > > --- a/mm/zsmalloc.c > > +++ b/mm/zsmalloc.c > > @@ -225,6 +225,12 @@ struct link_free { > > }; > > }; > > > > +struct zs_pool; > > + > > +struct zs_ops { > > + int (*evict)(struct zs_pool *pool, unsigned long handle); > > +}; > > + > > struct zs_pool { > > const char *name; > > > > @@ -242,6 +248,9 @@ struct zs_pool { > > #ifdef CONFIG_ZPOOL > > /* List tracking the zspages in LRU order by most recently added object */ > > struct list_head lru; > > + const struct zs_ops *ops; > > + struct zpool *zpool; > > + const struct zpool_ops *zpool_ops; > > #endif > > > > #ifdef CONFIG_ZSMALLOC_STAT > > @@ -385,6 +394,18 @@ static void record_obj(unsigned long handle, unsigned long obj) > > > > #ifdef CONFIG_ZPOOL > > > > +static int zs_zpool_evict(struct zs_pool *pool, unsigned long handle) > > +{ > > + if (pool->zpool && pool->zpool_ops && pool->zpool_ops->evict) > > + return pool->zpool_ops->evict(pool->zpool, handle); > > + else > > + return -ENOENT; > > +} > > + > > +static const struct zs_ops zs_zpool_ops = { > > + .evict = zs_zpool_evict > > +}; > > + > > static void *zs_zpool_create(const char *name, gfp_t gfp, > > const struct zpool_ops *zpool_ops, > > struct zpool *zpool) > > @@ -394,7 +415,19 @@ static void *zs_zpool_create(const char *name, gfp_t gfp, > > * different contexts and its caller must provide a valid > > * gfp mask. > > */ > > - return zs_create_pool(name); > > + struct zs_pool *pool = zs_create_pool(name); > > + > > + if (pool) { > > + pool->zpool = zpool; > > + pool->zpool_ops = zpool_ops; > > + > > + if (zpool_ops) > > I lost. When do we have zpool_ops as NULL? It actually can't be as of today, it's just the zpool API that pretends it can be, and so zsmalloc follows it here. Actually, this is pretty trivial to clean up across zpool and the drivers. How about the below patch? Nhat, would you mind picking that up into your series? It should be easy to update 4/5 in the same way. --- >From a608726b01ff78a8084f8255484c83d0aa144faf Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Thu, 17 Nov 2022 23:49:30 -0500 Subject: [PATCH] zpool: clean out dead code There is a lot of provision for flexibility that isn't actually needed or used: - Zswap, the only zpool user, always passes zpool_ops with an .evict method set. The backends who reclaim only do so for zswap, so they can also directly call zpool_ops without indirection or checks. - There is no need to check the retries parameters and bail with -EINVAL in the reclaim function, when that's called just a few lines below with a hard-coded 8. - There is no need to duplicate the evictable and sleep_mapped attrs from the driver in zpool, when driver is in zpool->driver. Signed-off-by: Johannes Weiner --- mm/z3fold.c | 36 +++++------------------------------- mm/zbud.c | 32 +++++--------------------------- mm/zpool.c | 9 ++------- 3 files changed, 12 insertions(+), 65 deletions(-) diff --git a/mm/z3fold.c b/mm/z3fold.c index cf71da10d04e..a4de0c317ac7 100644 --- a/mm/z3fold.c +++ b/mm/z3fold.c @@ -68,9 +68,6 @@ * Structures *****************/ struct z3fold_pool; -struct z3fold_ops { - int (*evict)(struct z3fold_pool *pool, unsigned long handle); -}; enum buddy { HEADLESS = 0, @@ -138,8 +135,6 @@ struct z3fold_header { * @stale: list of pages marked for freeing * @pages_nr: number of z3fold pages in the pool. * @c_handle: cache for z3fold_buddy_slots allocation - * @ops: pointer to a structure of user defined operations specified at - * pool creation time. * @zpool: zpool driver * @zpool_ops: zpool operations structure with an evict callback * @compact_wq: workqueue for page layout background optimization @@ -158,7 +153,6 @@ struct z3fold_pool { struct list_head stale; atomic64_t pages_nr; struct kmem_cache *c_handle; - const struct z3fold_ops *ops; struct zpool *zpool; const struct zpool_ops *zpool_ops; struct workqueue_struct *compact_wq; @@ -907,13 +901,11 @@ static inline struct z3fold_header *__z3fold_alloc(struct z3fold_pool *pool, * z3fold_create_pool() - create a new z3fold pool * @name: pool name * @gfp: gfp flags when allocating the z3fold pool structure - * @ops: user-defined operations for the z3fold pool * * Return: pointer to the new z3fold pool or NULL if the metadata allocation * failed. */ -static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp, - const struct z3fold_ops *ops) +static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp) { struct z3fold_pool *pool = NULL; int i, cpu; @@ -949,7 +941,6 @@ static struct z3fold_pool *z3fold_create_pool(const char *name, gfp_t gfp, if (!pool->release_wq) goto out_wq; INIT_WORK(&pool->work, free_pages_work); - pool->ops = ops; return pool; out_wq: @@ -1230,10 +1221,6 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) slots.pool = (unsigned long)pool | (1 << HANDLES_NOFREE); spin_lock(&pool->lock); - if (!pool->ops || !pool->ops->evict || retries == 0) { - spin_unlock(&pool->lock); - return -EINVAL; - } for (i = 0; i < retries; i++) { if (list_empty(&pool->lru)) { spin_unlock(&pool->lock); @@ -1319,17 +1306,17 @@ static int z3fold_reclaim_page(struct z3fold_pool *pool, unsigned int retries) } /* Issue the eviction callback(s) */ if (middle_handle) { - ret = pool->ops->evict(pool, middle_handle); + ret = pool->zpool_ops->evict(pool->zpool, middle_handle); if (ret) goto next; } if (first_handle) { - ret = pool->ops->evict(pool, first_handle); + ret = pool->zpool_ops->evict(pool->zpool, first_handle); if (ret) goto next; } if (last_handle) { - ret = pool->ops->evict(pool, last_handle); + ret = pool->zpool_ops->evict(pool->zpool, last_handle); if (ret) goto next; } @@ -1593,26 +1580,13 @@ static const struct movable_operations z3fold_mops = { * zpool ****************/ -static int z3fold_zpool_evict(struct z3fold_pool *pool, unsigned long handle) -{ - if (pool->zpool && pool->zpool_ops && pool->zpool_ops->evict) - return pool->zpool_ops->evict(pool->zpool, handle); - else - return -ENOENT; -} - -static const struct z3fold_ops z3fold_zpool_ops = { - .evict = z3fold_zpool_evict -}; - static void *z3fold_zpool_create(const char *name, gfp_t gfp, const struct zpool_ops *zpool_ops, struct zpool *zpool) { struct z3fold_pool *pool; - pool = z3fold_create_pool(name, gfp, - zpool_ops ? &z3fold_zpool_ops : NULL); + pool = z3fold_create_pool(name, gfp); if (pool) { pool->zpool = zpool; pool->zpool_ops = zpool_ops; diff --git a/mm/zbud.c b/mm/zbud.c index 6348932430b8..3acd26193920 100644 --- a/mm/zbud.c +++ b/mm/zbud.c @@ -74,10 +74,6 @@ struct zbud_pool; -struct zbud_ops { - int (*evict)(struct zbud_pool *pool, unsigned long handle); -}; - /** * struct zbud_pool - stores metadata for each zbud pool * @lock: protects all pool fields and first|last_chunk fields of any @@ -90,8 +86,6 @@ struct zbud_ops { * @lru: list tracking the zbud pages in LRU order by most recently * added buddy. * @pages_nr: number of zbud pages in the pool. - * @ops: pointer to a structure of user defined operations specified at - * pool creation time. * @zpool: zpool driver * @zpool_ops: zpool operations structure with an evict callback * @@ -110,7 +104,6 @@ struct zbud_pool { }; struct list_head lru; u64 pages_nr; - const struct zbud_ops *ops; struct zpool *zpool; const struct zpool_ops *zpool_ops; }; @@ -212,12 +205,11 @@ static int num_free_chunks(struct zbud_header *zhdr) /** * zbud_create_pool() - create a new zbud pool * @gfp: gfp flags when allocating the zbud pool structure - * @ops: user-defined operations for the zbud pool * * Return: pointer to the new zbud pool or NULL if the metadata allocation * failed. */ -static struct zbud_pool *zbud_create_pool(gfp_t gfp, const struct zbud_ops *ops) +static struct zbud_pool *zbud_create_pool(gfp_t gfp) { struct zbud_pool *pool; int i; @@ -231,7 +223,6 @@ static struct zbud_pool *zbud_create_pool(gfp_t gfp, const struct zbud_ops *ops) INIT_LIST_HEAD(&pool->buddied); INIT_LIST_HEAD(&pool->lru); pool->pages_nr = 0; - pool->ops = ops; return pool; } @@ -419,8 +410,7 @@ static int zbud_reclaim_page(struct zbud_pool *pool, unsigned int retries) unsigned long first_handle = 0, last_handle = 0; spin_lock(&pool->lock); - if (!pool->ops || !pool->ops->evict || list_empty(&pool->lru) || - retries == 0) { + if (list_empty(&pool->lru)) { spin_unlock(&pool->lock); return -EINVAL; } @@ -444,12 +434,12 @@ static int zbud_reclaim_page(struct zbud_pool *pool, unsigned int retries) /* Issue the eviction callback(s) */ if (first_handle) { - ret = pool->ops->evict(pool, first_handle); + ret = pool->zpool_ops->evict(pool->zpool, first_handle); if (ret) goto next; } if (last_handle) { - ret = pool->ops->evict(pool, last_handle); + ret = pool->zpool_ops->evict(pool->zpool, last_handle); if (ret) goto next; } @@ -524,25 +514,13 @@ static u64 zbud_get_pool_size(struct zbud_pool *pool) * zpool ****************/ -static int zbud_zpool_evict(struct zbud_pool *pool, unsigned long handle) -{ - if (pool->zpool && pool->zpool_ops && pool->zpool_ops->evict) - return pool->zpool_ops->evict(pool->zpool, handle); - else - return -ENOENT; -} - -static const struct zbud_ops zbud_zpool_ops = { - .evict = zbud_zpool_evict -}; - static void *zbud_zpool_create(const char *name, gfp_t gfp, const struct zpool_ops *zpool_ops, struct zpool *zpool) { struct zbud_pool *pool; - pool = zbud_create_pool(gfp, zpool_ops ? &zbud_zpool_ops : NULL); + pool = zbud_create_pool(gfp); if (pool) { pool->zpool = zpool; pool->zpool_ops = zpool_ops; diff --git a/mm/zpool.c b/mm/zpool.c index 68facc193496..74977526d60d 100644 --- a/mm/zpool.c +++ b/mm/zpool.c @@ -22,8 +22,6 @@ struct zpool { struct zpool_driver *driver; void *pool; const struct zpool_ops *ops; - bool evictable; - bool can_sleep_mapped; }; static LIST_HEAD(drivers_head); @@ -177,9 +175,6 @@ struct zpool *zpool_create_pool(const char *type, const char *name, gfp_t gfp, zpool->driver = driver; zpool->pool = driver->create(name, gfp, ops, zpool); - zpool->ops = ops; - zpool->evictable = driver->shrink && ops && ops->evict; - zpool->can_sleep_mapped = driver->sleep_mapped; if (!zpool->pool) { pr_err("couldn't create %s pool\n", type); @@ -380,7 +375,7 @@ u64 zpool_get_total_size(struct zpool *zpool) */ bool zpool_evictable(struct zpool *zpool) { - return zpool->evictable; + return zpool->driver->shrink; } /** @@ -391,7 +386,7 @@ bool zpool_evictable(struct zpool *zpool) */ bool zpool_can_sleep_mapped(struct zpool *zpool) { - return zpool->can_sleep_mapped; + return zpool->driver->sleep_mapped; } MODULE_LICENSE("GPL"); -- 2.38.1