From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx112.postini.com [74.125.245.112]) by kanga.kvack.org (Postfix) with SMTP id 024006B0005 for ; Thu, 11 Apr 2013 13:14:02 -0400 (EDT) MIME-Version: 1.0 Message-ID: Date: Thu, 11 Apr 2013 10:13:42 -0700 (PDT) From: Dan Magenheimer Subject: RE: [PATCH 02/10] staging: zcache: remove zcache_freeze References: <<1365553560-32258-1-git-send-email-liwanp@linux.vnet.ibm.com>> <<1365553560-32258-3-git-send-email-liwanp@linux.vnet.ibm.com>> In-Reply-To: <<1365553560-32258-3-git-send-email-liwanp@linux.vnet.ibm.com>> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Sender: owner-linux-mm@kvack.org List-ID: To: Wanpeng Li , Greg Kroah-Hartman Cc: Dan Magenheimer , Seth Jennings , Konrad Rzeszutek Wilk , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Bob Liu > From: Wanpeng Li [mailto:liwanp@linux.vnet.ibm.com] > Subject: [PATCH 02/10] staging: zcache: remove zcache_freeze >=20 > The default value of zcache_freeze is false and it won't be modified by > other codes. Remove zcache_freeze since no routine can disable zcache > during system running. >=20 > Signed-off-by: Wanpeng Li I'd prefer to leave this code in place as it may be very useful if/when zcache becomes more tightly integrated into the MM subsystem and the rest of the kernel. And the subtleties for temporarily disabling zcache (which is what zcache_freeze does) are non-obvious and may cause data loss so if someone wants to add this functionality back in later and don't have this piece of code, it may take a lot of pain to get it working. Usage example: All CPUs are fully saturated so it is questionable whether spending CPU cycles for compression is wise. Kernel could disable zcache using zcache_freeze. (Yes, a new entry point would need to be added to enable/disable zcache_freeze.) My two cents... others are welcome to override. > --- > drivers/staging/zcache/zcache-main.c | 55 +++++++++++-----------------= ------ > 1 file changed, 18 insertions(+), 37 deletions(-) >=20 > diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcach= e/zcache-main.c > index e23d814..fe6801a 100644 > --- a/drivers/staging/zcache/zcache-main.c > +++ b/drivers/staging/zcache/zcache-main.c > @@ -1118,15 +1118,6 @@ free_and_out: > #endif /* CONFIG_ZCACHE_WRITEBACK */ >=20 > /* > - * When zcache is disabled ("frozen"), pools can be created and destroye= d, > - * but all puts (and thus all other operations that require memory alloc= ation) > - * must fail. If zcache is unfrozen, accepts puts, then frozen again, > - * data consistency requires all puts while frozen to be converted into > - * flushes. > - */ > -static bool zcache_freeze; > - > -/* > * This zcache shrinker interface reduces the number of ephemeral pagefr= ames > * used by zcache to approximately the same as the total number of LRU_F= ILE > * pageframes in use, and now also reduces the number of persistent page= frames > @@ -1221,44 +1212,34 @@ int zcache_put_page(int cli_id, int pool_id, stru= ct tmem_oid *oidp, > { > =09struct tmem_pool *pool; > =09struct tmem_handle th; > -=09int ret =3D -1; > +=09int ret =3D 0; > =09void *pampd =3D NULL; >=20 > =09BUG_ON(!irqs_disabled()); > =09pool =3D zcache_get_pool_by_id(cli_id, pool_id); > =09if (unlikely(pool =3D=3D NULL)) > =09=09goto out; > -=09if (!zcache_freeze) { > -=09=09ret =3D 0; > -=09=09th.client_id =3D cli_id; > -=09=09th.pool_id =3D pool_id; > -=09=09th.oid =3D *oidp; > -=09=09th.index =3D index; > -=09=09pampd =3D zcache_pampd_create((char *)page, size, raw, > -=09=09=09=09ephemeral, &th); > -=09=09if (pampd =3D=3D NULL) { > -=09=09=09ret =3D -ENOMEM; > -=09=09=09if (ephemeral) > -=09=09=09=09inc_zcache_failed_eph_puts(); > -=09=09=09else > -=09=09=09=09inc_zcache_failed_pers_puts(); > -=09=09} else { > -=09=09=09if (ramster_enabled) > -=09=09=09=09ramster_do_preload_flnode(pool); > -=09=09=09ret =3D tmem_put(pool, oidp, index, 0, pampd); > -=09=09=09if (ret < 0) > -=09=09=09=09BUG(); > -=09=09} > -=09=09zcache_put_pool(pool); > + > +=09th.client_id =3D cli_id; > +=09th.pool_id =3D pool_id; > +=09th.oid =3D *oidp; > +=09th.index =3D index; > +=09pampd =3D zcache_pampd_create((char *)page, size, raw, > +=09=09=09ephemeral, &th); > +=09if (pampd =3D=3D NULL) { > +=09=09ret =3D -ENOMEM; > +=09=09if (ephemeral) > +=09=09=09inc_zcache_failed_eph_puts(); > +=09=09else > +=09=09=09inc_zcache_failed_pers_puts(); > =09} else { > -=09=09inc_zcache_put_to_flush(); > =09=09if (ramster_enabled) > =09=09=09ramster_do_preload_flnode(pool); > -=09=09if (atomic_read(&pool->obj_count) > 0) > -=09=09=09/* the put fails whether the flush succeeds or not */ > -=09=09=09(void)tmem_flush_page(pool, oidp, index); > -=09=09zcache_put_pool(pool); > +=09=09ret =3D tmem_put(pool, oidp, index, 0, pampd); > +=09=09if (ret < 0) > +=09=09=09BUG(); > =09} > +=09zcache_put_pool(pool); > out: > =09return ret; > } > -- > 1.7.10.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932474Ab3DKROK (ORCPT ); Thu, 11 Apr 2013 13:14:10 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:21104 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753710Ab3DKROH convert rfc822-to-8bit (ORCPT ); Thu, 11 Apr 2013 13:14:07 -0400 MIME-Version: 1.0 Message-ID: Date: Thu, 11 Apr 2013 10:13:42 -0700 (PDT) From: Dan Magenheimer To: Wanpeng Li , Greg Kroah-Hartman Cc: Dan Magenheimer , Seth Jennings , Konrad Rzeszutek Wilk , Minchan Kim , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Bob Liu Subject: RE: [PATCH 02/10] staging: zcache: remove zcache_freeze References: <<1365553560-32258-1-git-send-email-liwanp@linux.vnet.ibm.com>> <<1365553560-32258-3-git-send-email-liwanp@linux.vnet.ibm.com>> In-Reply-To: <<1365553560-32258-3-git-send-email-liwanp@linux.vnet.ibm.com>> X-Priority: 3 X-Mailer: Oracle Beehive Extensions for Outlook 2.0.1.7 (607090) [OL 12.0.6668.5000 (x86)] Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8BIT X-Source-IP: acsinet21.oracle.com [141.146.126.237] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > From: Wanpeng Li [mailto:liwanp@linux.vnet.ibm.com] > Subject: [PATCH 02/10] staging: zcache: remove zcache_freeze > > The default value of zcache_freeze is false and it won't be modified by > other codes. Remove zcache_freeze since no routine can disable zcache > during system running. > > Signed-off-by: Wanpeng Li I'd prefer to leave this code in place as it may be very useful if/when zcache becomes more tightly integrated into the MM subsystem and the rest of the kernel. And the subtleties for temporarily disabling zcache (which is what zcache_freeze does) are non-obvious and may cause data loss so if someone wants to add this functionality back in later and don't have this piece of code, it may take a lot of pain to get it working. Usage example: All CPUs are fully saturated so it is questionable whether spending CPU cycles for compression is wise. Kernel could disable zcache using zcache_freeze. (Yes, a new entry point would need to be added to enable/disable zcache_freeze.) My two cents... others are welcome to override. > --- > drivers/staging/zcache/zcache-main.c | 55 +++++++++++----------------------- > 1 file changed, 18 insertions(+), 37 deletions(-) > > diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c > index e23d814..fe6801a 100644 > --- a/drivers/staging/zcache/zcache-main.c > +++ b/drivers/staging/zcache/zcache-main.c > @@ -1118,15 +1118,6 @@ free_and_out: > #endif /* CONFIG_ZCACHE_WRITEBACK */ > > /* > - * When zcache is disabled ("frozen"), pools can be created and destroyed, > - * but all puts (and thus all other operations that require memory allocation) > - * must fail. If zcache is unfrozen, accepts puts, then frozen again, > - * data consistency requires all puts while frozen to be converted into > - * flushes. > - */ > -static bool zcache_freeze; > - > -/* > * This zcache shrinker interface reduces the number of ephemeral pageframes > * used by zcache to approximately the same as the total number of LRU_FILE > * pageframes in use, and now also reduces the number of persistent pageframes > @@ -1221,44 +1212,34 @@ int zcache_put_page(int cli_id, int pool_id, struct tmem_oid *oidp, > { > struct tmem_pool *pool; > struct tmem_handle th; > - int ret = -1; > + int ret = 0; > void *pampd = NULL; > > BUG_ON(!irqs_disabled()); > pool = zcache_get_pool_by_id(cli_id, pool_id); > if (unlikely(pool == NULL)) > goto out; > - if (!zcache_freeze) { > - ret = 0; > - th.client_id = cli_id; > - th.pool_id = pool_id; > - th.oid = *oidp; > - th.index = index; > - pampd = zcache_pampd_create((char *)page, size, raw, > - ephemeral, &th); > - if (pampd == NULL) { > - ret = -ENOMEM; > - if (ephemeral) > - inc_zcache_failed_eph_puts(); > - else > - inc_zcache_failed_pers_puts(); > - } else { > - if (ramster_enabled) > - ramster_do_preload_flnode(pool); > - ret = tmem_put(pool, oidp, index, 0, pampd); > - if (ret < 0) > - BUG(); > - } > - zcache_put_pool(pool); > + > + th.client_id = cli_id; > + th.pool_id = pool_id; > + th.oid = *oidp; > + th.index = index; > + pampd = zcache_pampd_create((char *)page, size, raw, > + ephemeral, &th); > + if (pampd == NULL) { > + ret = -ENOMEM; > + if (ephemeral) > + inc_zcache_failed_eph_puts(); > + else > + inc_zcache_failed_pers_puts(); > } else { > - inc_zcache_put_to_flush(); > if (ramster_enabled) > ramster_do_preload_flnode(pool); > - if (atomic_read(&pool->obj_count) > 0) > - /* the put fails whether the flush succeeds or not */ > - (void)tmem_flush_page(pool, oidp, index); > - zcache_put_pool(pool); > + ret = tmem_put(pool, oidp, index, 0, pampd); > + if (ret < 0) > + BUG(); > } > + zcache_put_pool(pool); > out: > return ret; > } > -- > 1.7.10.4