| Message ID | 20240928021620.8369-4-kanchana.p.sridhar@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: zswap swap-out of large folios | expand |
On Fri, Sep 27, 2024 at 7:16 PM Kanchana P Sridhar <kanchana.p.sridhar@intel.com> wrote: > > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > to be representative of the call it makes to percpu_ref_tryget(). > A subsequent patch will introduce a new zswap_pool_get() that calls > percpu_ref_get(). > > The intent behind this change is for higher level zswap API such as > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > refcount is "0" (which means it could be getting destroyed) and to handle > this as an error condition. zswap_store() would proceed only if > zswap_pool_tryget() returns success, and any additional pool refcounts that > need to be obtained for compressing sub-pages in a large folio could simply > call zswap_pool_get(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> Acked-by: Yosry Ahmed <yosryahmed@google.com> > --- > mm/zswap.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index fd7a8c14457a..0f281e50a034 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -403,7 +403,7 @@ static void __zswap_pool_empty(struct percpu_ref *ref) > spin_unlock_bh(&zswap_pools_lock); > } > > -static int __must_check zswap_pool_get(struct zswap_pool *pool) > +static int __must_check zswap_pool_tryget(struct zswap_pool *pool) > { > if (!pool) > return 0; > @@ -441,7 +441,7 @@ static struct zswap_pool *zswap_pool_current_get(void) > rcu_read_lock(); > > pool = __zswap_pool_current(); > - if (!zswap_pool_get(pool)) > + if (!zswap_pool_tryget(pool)) > pool = NULL; > > rcu_read_unlock(); > @@ -462,7 +462,7 @@ static struct zswap_pool *zswap_pool_find_get(char *type, char *compressor) > if (strcmp(zpool_get_type(pool->zpool), type)) > continue; > /* if we can't get it, it's about to be destroyed */ > - if (!zswap_pool_get(pool)) > + if (!zswap_pool_tryget(pool)) > continue; > return pool; > } > -- > 2.27.0 >
On 2024/9/28 10:16, Kanchana P Sridhar wrote: > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > to be representative of the call it makes to percpu_ref_tryget(). > A subsequent patch will introduce a new zswap_pool_get() that calls > percpu_ref_get(). > > The intent behind this change is for higher level zswap API such as > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > refcount is "0" (which means it could be getting destroyed) and to handle > this as an error condition. zswap_store() would proceed only if > zswap_pool_tryget() returns success, and any additional pool refcounts that > need to be obtained for compressing sub-pages in a large folio could simply > call zswap_pool_get(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> > --- > mm/zswap.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index fd7a8c14457a..0f281e50a034 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -403,7 +403,7 @@ static void __zswap_pool_empty(struct percpu_ref *ref) > spin_unlock_bh(&zswap_pools_lock); > } > > -static int __must_check zswap_pool_get(struct zswap_pool *pool) > +static int __must_check zswap_pool_tryget(struct zswap_pool *pool) > { > if (!pool) > return 0; > @@ -441,7 +441,7 @@ static struct zswap_pool *zswap_pool_current_get(void) > rcu_read_lock(); > > pool = __zswap_pool_current(); > - if (!zswap_pool_get(pool)) > + if (!zswap_pool_tryget(pool)) > pool = NULL; > > rcu_read_unlock(); > @@ -462,7 +462,7 @@ static struct zswap_pool *zswap_pool_find_get(char *type, char *compressor) > if (strcmp(zpool_get_type(pool->zpool), type)) > continue; > /* if we can't get it, it's about to be destroyed */ > - if (!zswap_pool_get(pool)) > + if (!zswap_pool_tryget(pool)) > continue; > return pool; > }
On Fri, Sep 27, 2024 at 07:16:15PM -0700, Kanchana P Sridhar wrote: > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > to be representative of the call it makes to percpu_ref_tryget(). > A subsequent patch will introduce a new zswap_pool_get() that calls > percpu_ref_get(). > > The intent behind this change is for higher level zswap API such as > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > refcount is "0" (which means it could be getting destroyed) and to handle > this as an error condition. zswap_store() would proceed only if > zswap_pool_tryget() returns success, and any additional pool refcounts that > need to be obtained for compressing sub-pages in a large folio could simply > call zswap_pool_get(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
On Fri, Sep 27, 2024 at 7:16 PM Kanchana P Sridhar <kanchana.p.sridhar@intel.com> wrote: > > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > to be representative of the call it makes to percpu_ref_tryget(). > A subsequent patch will introduce a new zswap_pool_get() that calls > percpu_ref_get(). > > The intent behind this change is for higher level zswap API such as > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > refcount is "0" (which means it could be getting destroyed) and to handle > this as an error condition. zswap_store() would proceed only if > zswap_pool_tryget() returns success, and any additional pool refcounts that > need to be obtained for compressing sub-pages in a large folio could simply > call zswap_pool_get(). > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> Reviewed-by: Nhat Pham <nphamcs@gmail.com>
> -----Original Message----- > From: Chengming Zhou <chengming.zhou@linux.dev> > Sent: Friday, September 27, 2024 10:44 PM > To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>; linux- > kernel@vger.kernel.org; linux-mm@kvack.org; hannes@cmpxchg.org; > yosryahmed@google.com; nphamcs@gmail.com; usamaarif642@gmail.com; > shakeel.butt@linux.dev; ryan.roberts@arm.com; Huang, Ying > <ying.huang@intel.com>; 21cnbao@gmail.com; akpm@linux-foundation.org > Cc: Zou, Nanhai <nanhai.zou@intel.com>; Feghali, Wajdi K > <wajdi.k.feghali@intel.com>; Gopal, Vinodh <vinodh.gopal@intel.com> > Subject: Re: [PATCH v8 3/8] mm: zswap: Rename zswap_pool_get() to > zswap_pool_tryget(). > > On 2024/9/28 10:16, Kanchana P Sridhar wrote: > > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > > to be representative of the call it makes to percpu_ref_tryget(). > > A subsequent patch will introduce a new zswap_pool_get() that calls > > percpu_ref_get(). > > > > The intent behind this change is for higher level zswap API such as > > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > > refcount is "0" (which means it could be getting destroyed) and to handle > > this as an error condition. zswap_store() would proceed only if > > zswap_pool_tryget() returns success, and any additional pool refcounts that > > need to be obtained for compressing sub-pages in a large folio could simply > > call zswap_pool_get(). > > > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> > > Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> Thanks Chengming! Thanks, Kanchana > > > --- > > mm/zswap.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index fd7a8c14457a..0f281e50a034 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -403,7 +403,7 @@ static void __zswap_pool_empty(struct percpu_ref > *ref) > > spin_unlock_bh(&zswap_pools_lock); > > } > > > > -static int __must_check zswap_pool_get(struct zswap_pool *pool) > > +static int __must_check zswap_pool_tryget(struct zswap_pool *pool) > > { > > if (!pool) > > return 0; > > @@ -441,7 +441,7 @@ static struct zswap_pool > *zswap_pool_current_get(void) > > rcu_read_lock(); > > > > pool = __zswap_pool_current(); > > - if (!zswap_pool_get(pool)) > > + if (!zswap_pool_tryget(pool)) > > pool = NULL; > > > > rcu_read_unlock(); > > @@ -462,7 +462,7 @@ static struct zswap_pool > *zswap_pool_find_get(char *type, char *compressor) > > if (strcmp(zpool_get_type(pool->zpool), type)) > > continue; > > /* if we can't get it, it's about to be destroyed */ > > - if (!zswap_pool_get(pool)) > > + if (!zswap_pool_tryget(pool)) > > continue; > > return pool; > > }
> -----Original Message----- > From: Nhat Pham <nphamcs@gmail.com> > Sent: Saturday, September 28, 2024 4:27 PM > To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com> > Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org; > hannes@cmpxchg.org; yosryahmed@google.com; > chengming.zhou@linux.dev; usamaarif642@gmail.com; > shakeel.butt@linux.dev; ryan.roberts@arm.com; Huang, Ying > <ying.huang@intel.com>; 21cnbao@gmail.com; akpm@linux-foundation.org; > Zou, Nanhai <nanhai.zou@intel.com>; Feghali, Wajdi K > <wajdi.k.feghali@intel.com>; Gopal, Vinodh <vinodh.gopal@intel.com> > Subject: Re: [PATCH v8 3/8] mm: zswap: Rename zswap_pool_get() to > zswap_pool_tryget(). > > On Fri, Sep 27, 2024 at 7:16 PM Kanchana P Sridhar > <kanchana.p.sridhar@intel.com> wrote: > > > > Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() > > to be representative of the call it makes to percpu_ref_tryget(). > > A subsequent patch will introduce a new zswap_pool_get() that calls > > percpu_ref_get(). > > > > The intent behind this change is for higher level zswap API such as > > zswap_store() to call zswap_pool_tryget() to check upfront if the pool's > > refcount is "0" (which means it could be getting destroyed) and to handle > > this as an error condition. zswap_store() would proceed only if > > zswap_pool_tryget() returns success, and any additional pool refcounts that > > need to be obtained for compressing sub-pages in a large folio could simply > > call zswap_pool_get(). > > > > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> > > Reviewed-by: Nhat Pham <nphamcs@gmail.com> Thanks Nhat! Thanks, Kanchana
diff --git a/mm/zswap.c b/mm/zswap.c index fd7a8c14457a..0f281e50a034 100644 --- a/mm/zswap.c +++ b/mm/zswap.c @@ -403,7 +403,7 @@ static void __zswap_pool_empty(struct percpu_ref *ref) spin_unlock_bh(&zswap_pools_lock); } -static int __must_check zswap_pool_get(struct zswap_pool *pool) +static int __must_check zswap_pool_tryget(struct zswap_pool *pool) { if (!pool) return 0; @@ -441,7 +441,7 @@ static struct zswap_pool *zswap_pool_current_get(void) rcu_read_lock(); pool = __zswap_pool_current(); - if (!zswap_pool_get(pool)) + if (!zswap_pool_tryget(pool)) pool = NULL; rcu_read_unlock(); @@ -462,7 +462,7 @@ static struct zswap_pool *zswap_pool_find_get(char *type, char *compressor) if (strcmp(zpool_get_type(pool->zpool), type)) continue; /* if we can't get it, it's about to be destroyed */ - if (!zswap_pool_get(pool)) + if (!zswap_pool_tryget(pool)) continue; return pool; }
Modify the name of the existing zswap_pool_get() to zswap_pool_tryget() to be representative of the call it makes to percpu_ref_tryget(). A subsequent patch will introduce a new zswap_pool_get() that calls percpu_ref_get(). The intent behind this change is for higher level zswap API such as zswap_store() to call zswap_pool_tryget() to check upfront if the pool's refcount is "0" (which means it could be getting destroyed) and to handle this as an error condition. zswap_store() would proceed only if zswap_pool_tryget() returns success, and any additional pool refcounts that need to be obtained for compressing sub-pages in a large folio could simply call zswap_pool_get(). Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@intel.com> --- mm/zswap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)