Re: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog functionality

["Dilger, Andreas" <andreas.dilger@intel.com>]

On 2015/12/23, 14:40, "Simmons, James A." <simmonsja@ornl.gov> wrote:

>>From: Niranjan Dighe <ndighe@visteon.com>
>>
>>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.

Cheers, Andreas

>
>
>Signed-off-by: Niranjan Dighe <ndighe@visteon.com>
>---
> .../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