Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757249AbbLWWps (ORCPT ); Wed, 23 Dec 2015 17:45:48 -0500 Received: from mta02.ornl.gov ([128.219.177.12]:56634 "EHLO mta02.ornl.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754690AbbLWWpr convert rfc822-to-8bit (ORCPT ); Wed, 23 Dec 2015 17:45:47 -0500 X-SG: RELAYLIST X-IronPort-AV: E=Sophos;i="5.20,471,1444708800"; d="scan'208";a="94838446" From: "Simmons, James A." To: "'Dilger, Andreas'" , "'Dighe, Niranjan (N.)'" , "Drokin, Oleg" , "Greg Kroah-Hartman" , "Eremin, Dmitry" , James Simmons , "Mike Rapoport" , "Patrick Boettcher" , Matthew Tyler CC: "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "lustre-devel@lists.lustre.org" Subject: RE: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog functionality Thread-Topic: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog functionality Thread-Index: AQHRPcxfl7DBtjj9fEqkVKZCm6W3NJ7ZKPFw Date: Wed, 23 Dec 2015 22:45:43 +0000 Message-ID: References: <20151223094014.GA11106@siberianhusky> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [128.219.12.132] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8429 Lines: 303 >On 2015/12/23, 14:40, "Simmons, James A." wrote: > >>>From: Niranjan Dighe >>> >>>Remove IOC_LIBCFS_MEMHOG ioctl functionality as it is no longer needed >>>thereby >>>making functions like - kportal_memhog_alloc(), kportal_memhog_free() >>>and type - >>>struct libcfs_device_userstate unused. >> >>Actually so much more can be cleaned up. This work is also being done at >> >>http://review.whamcloud.com/#/c/17492. >> >>I will update that patch and post it here as well. > >James, part of that patch is for the userspace tools, which isn't >applicable here. Also, it probably makes sense to delete the panic injection as a >separate patch, so I think this patch is good as-is for removing memhog. After looking at the code I agree. At first I thought it would be a simple cleanup expansion of this patch but I see a lot more cleanups coming after this. This work is the first step to removing the cfs_psdev_* abstraction. I will submit the cleanups soon. For now this patch can be merged. Acked-by: James Simmons > > >Signed-off-by: Niranjan Dighe >--- > .../lustre/include/linux/libcfs/libcfs_private.h | 5 - > .../lustre/lustre/libcfs/linux/linux-module.c | 14 +- > drivers/staging/lustre/lustre/libcfs/module.c | 139 >-------------------- > 3 files changed, 2 insertions(+), 156 deletions(-) > >diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h >b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h >index d6273e1..e044d6f 100644 >--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h >+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h >@@ -391,11 +391,6 @@ int cfs_percpt_atomic_summary(atomic_t **refs); > * Support for temporary event tracing with minimal Heisenberg effect. > * -------------------------------------------------------------------- >*/ > >-struct libcfs_device_userstate { >- int ldu_memhog_pages; >- struct page *ldu_memhog_root_page; >-}; >- > #define MKSTR(ptr) ((ptr)) ? (ptr) : "" > > static inline int cfs_size_round4(int val) >diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c >b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c >index 70a99cf0..eccfe8bd 100644 >--- a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c >+++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c >@@ -98,14 +98,12 @@ int libcfs_ioctl_popdata(void *arg, void *data, int >size) > static int > libcfs_psdev_open(struct inode *inode, struct file *file) > { >- struct libcfs_device_userstate **pdu = NULL; > int rc = 0; > > if (!inode) > return -EINVAL; >- pdu = (struct libcfs_device_userstate **)&file->private_data; > if (libcfs_psdev_ops.p_open != NULL) >- rc = libcfs_psdev_ops.p_open(0, (void *)pdu); >+ rc = libcfs_psdev_ops.p_open(0, NULL); > else > return -EPERM; > return rc; >@@ -115,14 +113,12 @@ libcfs_psdev_open(struct inode *inode, struct file >*file) > static int > libcfs_psdev_release(struct inode *inode, struct file *file) > { >- struct libcfs_device_userstate *pdu; > int rc = 0; > > if (!inode) > return -EINVAL; >- pdu = file->private_data; > if (libcfs_psdev_ops.p_close != NULL) >- rc = libcfs_psdev_ops.p_close(0, (void *)pdu); >+ rc = libcfs_psdev_ops.p_close(0, NULL); > else > rc = -EPERM; > return rc; >@@ -152,14 +148,8 @@ static long libcfs_ioctl(struct file *file, > return -EPERM; > panic("debugctl-invoked panic"); > return 0; >- case IOC_LIBCFS_MEMHOG: >- if (!capable(CFS_CAP_SYS_ADMIN)) >- return -EPERM; >- /* go thought */ > } > >- pfile.off = 0; >- pfile.private_data = file->private_data; > if (libcfs_psdev_ops.p_ioctl != NULL) > rc = libcfs_psdev_ops.p_ioctl(&pfile, cmd, (void *)arg); > else >diff --git a/drivers/staging/lustre/lustre/libcfs/module.c >b/drivers/staging/lustre/lustre/libcfs/module.c >index 329d78c..0067e53 100644 >--- a/drivers/staging/lustre/lustre/libcfs/module.c >+++ b/drivers/staging/lustre/lustre/libcfs/module.c >@@ -68,142 +68,16 @@ MODULE_LICENSE("GPL"); > > static struct dentry *lnet_debugfs_root; > >-static void kportal_memhog_free(struct libcfs_device_userstate *ldu) >-{ >- struct page **level0p = &ldu->ldu_memhog_root_page; >- struct page **level1p; >- struct page **level2p; >- int count1; >- int count2; >- >- if (*level0p != NULL) { >- >- level1p = (struct page **)page_address(*level0p); >- count1 = 0; >- >- while (count1 < PAGE_CACHE_SIZE/sizeof(struct page *) && >- *level1p != NULL) { >- >- level2p = (struct page **)page_address(*level1p); >- count2 = 0; >- >- while (count2 < PAGE_CACHE_SIZE/sizeof(struct page *) && >- *level2p != NULL) { >- >- __free_page(*level2p); >- ldu->ldu_memhog_pages--; >- level2p++; >- count2++; >- } >- >- __free_page(*level1p); >- ldu->ldu_memhog_pages--; >- level1p++; >- count1++; >- } >- >- __free_page(*level0p); >- ldu->ldu_memhog_pages--; >- >- *level0p = NULL; >- } >- >- LASSERT(ldu->ldu_memhog_pages == 0); >-} >- >-static int kportal_memhog_alloc(struct libcfs_device_userstate *ldu, int >npages, >- gfp_t flags) >-{ >- struct page **level0p; >- struct page **level1p; >- struct page **level2p; >- int count1; >- int count2; >- >- LASSERT(ldu->ldu_memhog_pages == 0); >- LASSERT(ldu->ldu_memhog_root_page == NULL); >- >- if (npages < 0) >- return -EINVAL; >- >- if (npages == 0) >- return 0; >- >- level0p = &ldu->ldu_memhog_root_page; >- *level0p = alloc_page(flags); >- if (*level0p == NULL) >- return -ENOMEM; >- ldu->ldu_memhog_pages++; >- >- level1p = (struct page **)page_address(*level0p); >- count1 = 0; >- memset(level1p, 0, PAGE_CACHE_SIZE); >- >- while (ldu->ldu_memhog_pages < npages && >- count1 < PAGE_CACHE_SIZE/sizeof(struct page *)) { >- >- if (cfs_signal_pending()) >- return -EINTR; >- >- *level1p = alloc_page(flags); >- if (*level1p == NULL) >- return -ENOMEM; >- ldu->ldu_memhog_pages++; >- >- level2p = (struct page **)page_address(*level1p); >- count2 = 0; >- memset(level2p, 0, PAGE_CACHE_SIZE); >- >- while (ldu->ldu_memhog_pages < npages && >- count2 < PAGE_CACHE_SIZE/sizeof(struct page *)) { >- >- if (cfs_signal_pending()) >- return -EINTR; >- >- *level2p = alloc_page(flags); >- if (*level2p == NULL) >- return -ENOMEM; >- ldu->ldu_memhog_pages++; >- >- level2p++; >- count2++; >- } >- >- level1p++; >- count1++; >- } >- >- return 0; >-} >- > /* called when opening /dev/device */ > static int libcfs_psdev_open(unsigned long flags, void *args) > { >- struct libcfs_device_userstate *ldu; >- > try_module_get(THIS_MODULE); >- >- LIBCFS_ALLOC(ldu, sizeof(*ldu)); >- if (ldu != NULL) { >- ldu->ldu_memhog_pages = 0; >- ldu->ldu_memhog_root_page = NULL; >- } >- *(struct libcfs_device_userstate **)args = ldu; >- > return 0; > } > > /* called when closing /dev/device */ > static int libcfs_psdev_release(unsigned long flags, void *args) > { >- struct libcfs_device_userstate *ldu; >- >- ldu = (struct libcfs_device_userstate *)args; >- if (ldu != NULL) { >- kportal_memhog_free(ldu); >- LIBCFS_FREE(ldu, sizeof(*ldu)); >- } >- > module_put(THIS_MODULE); > return 0; > } >@@ -260,19 +134,6 @@ static int libcfs_ioctl_int(struct cfs_psdev_file >*pfile, unsigned long cmd, > return -EINVAL; > libcfs_debug_mark_buffer(data->ioc_inlbuf1); > return 0; >- case IOC_LIBCFS_MEMHOG: >- if (pfile->private_data == NULL) { >- err = -EINVAL; >- } else { >- kportal_memhog_free(pfile->private_data); >- /* XXX The ioc_flags is not GFP flags now, need to be fixed */ >- err = kportal_memhog_alloc(pfile->private_data, >- data->ioc_count, >- data->ioc_flags); >- if (err != 0) >- kportal_memhog_free(pfile->private_data); >- } >- break; > > default: { > struct libcfs_ioctl_handler *hand; >-- >1.7.9.5 >_______________________________________________ >lustre-devel mailing list >lustre-devel@lists.lustre.org >http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org > > Cheers, Andreas -- Andreas Dilger Lustre Principal Architect Intel High Performance Data Division -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/