2015-12-23 09:55:09

by Dighe, Niranjan (N.)

[permalink] [raw]
Subject: [PATCH] staging: lustre: Remove unused memhog functionality

From: Niranjan Dighe <[email protected]>

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.

Signed-off-by: Niranjan Dighe <[email protected]>
---
.../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


2015-12-23 21:40:29

by Simmons, James A.

[permalink] [raw]
Subject: RE: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog functionality

>From: Niranjan Dighe <[email protected]>
>
>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.


Signed-off-by: Niranjan Dighe <[email protected]>
---
.../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
[email protected]
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

2015-12-23 21:48:50

by Dilger, Andreas

[permalink] [raw]
Subject: Re: [PATCH] staging: lustre: Remove unused memhog functionality

On 2015/12/23, 02:40, "Dighe, Niranjan (N.)" <[email protected]> wrote:

>From: Niranjan Dighe <[email protected]>
>
>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.
>
>Signed-off-by: Niranjan Dighe <[email protected]>

Thanks for the patch.

Reviewed-by: Andreas Dilger <[email protected]>

>---
> .../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
>


Cheers, Andreas
--
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division

2015-12-23 21:53:32

by Dilger, Andreas

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

On 2015/12/23, 14:40, "Simmons, James A." <[email protected]> wrote:

>>From: Niranjan Dighe <[email protected]>
>>
>>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 <[email protected]>
>---
> .../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
>[email protected]
>http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
>
>


Cheers, Andreas
--
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division

2015-12-23 22:45:48

by Simmons, James A.

[permalink] [raw]
Subject: RE: [lustre-devel] [PATCH] staging: lustre: Remove unused memhog functionality

>On 2015/12/23, 14:40, "Simmons, James A." <[email protected]> wrote:
>
>>>From: Niranjan Dighe <[email protected]>
>>>
>>>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 <[email protected]>

>
>
>Signed-off-by: Niranjan Dighe <[email protected]>
>---
> .../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
>[email protected]
>http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
>
>


Cheers, Andreas
--
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division