[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ef0eb906e5064614a30507e256d0a58b@EXCHCS32.ornl.gov>
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