lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Wed, 23 Dec 2015 22:45:43 +0000
From:	"Simmons, James A." <simmonsja@...l.gov>
To:	"'Dilger, Andreas'" <andreas.dilger@...el.com>,
	"'Dighe, Niranjan (N.)'" <ndighe@...teon.com>,
	"Drokin, Oleg" <oleg.drokin@...el.com>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	"Eremin, Dmitry" <dmitry.eremin@...el.com>,
	James Simmons <jsimmons@...radead.org>,
	"Mike Rapoport" <mike.rapoport@...il.com>,
	"Patrick Boettcher" <patrick.boettcher@...teo.de>,
	Matthew Tyler <matt.tyler@...shics.com>
CC:	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lustre-devel@...ts.lustre.org" <lustre-devel@...ts.lustre.org>
Subject: RE: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog
 functionality

>On 2015/12/23, 14:40, "Simmons, James A." <simmonsja@...l.gov> wrote:
>
>>>From: Niranjan Dighe <ndighe@...teon.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.

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 <jsimmons@...radead.org>

>
>
>Signed-off-by: Niranjan Dighe <ndighe@...teon.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@...ts.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ