* [RFC 0/2] enable pool shrinking in page unit @ 2014-10-23 12:52 Gioh Kim 2014-10-23 12:52 ` [RFC 1/2] staging: ion: shrink page-pool by " Gioh Kim 2014-10-23 12:52 ` [RFC 2/2] staging: ion: debugfs to shrink pool Gioh Kim 0 siblings, 2 replies; 9+ messages in thread From: Gioh Kim @ 2014-10-23 12:52 UTC (permalink / raw) To: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin Cc: devel, linux-kernel, gunho.lee, gioh.kim Hello, Current shrinking is not page unit, block unit. But shrinker returns the pools size with page unit. So it is confused. And there is no ways to shrink at any moment and any amount. I have 2 patches: 1. Patch 1/2: make pool be shrinked by page unit This patch trys to shrink pool in page unit. 2. Patch 2/2: enable debugfs to shrink pool directly This patch enable debugfs to specify shrink amount. Thanks for any feedback. This patchset is based on linux-next-20141023. Gioh Kim (2): staging: ion: shrink page-pool by page unit staging: ion: debugfs to shrink pool drivers/staging/android/ion/ion.c | 31 +++++++------------------ drivers/staging/android/ion/ion_page_pool.c | 5 ++-- drivers/staging/android/ion/ion_system_heap.c | 7 ++++-- 3 files changed, 16 insertions(+), 27 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 1/2] staging: ion: shrink page-pool by page unit 2014-10-23 12:52 [RFC 0/2] enable pool shrinking in page unit Gioh Kim @ 2014-10-23 12:52 ` Gioh Kim 2014-10-23 20:36 ` Devendra Naga 2014-10-23 12:52 ` [RFC 2/2] staging: ion: debugfs to shrink pool Gioh Kim 1 sibling, 1 reply; 9+ messages in thread From: Gioh Kim @ 2014-10-23 12:52 UTC (permalink / raw) To: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin Cc: devel, linux-kernel, gunho.lee, gioh.kim This patch shrink page-pool by page unit. Signed-off-by: Gioh Kim <gioh.kim@lge.com> --- drivers/staging/android/ion/ion_page_pool.c | 5 +++-- drivers/staging/android/ion/ion_system_heap.c | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index 5864f3d..165152f 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -116,7 +116,7 @@ static int ion_page_pool_total(struct ion_page_pool *pool, bool high) int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask, int nr_to_scan) { - int freed; + int freed = 0; bool high; if (current_is_kswapd()) @@ -127,7 +127,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask, if (nr_to_scan == 0) return ion_page_pool_total(pool, high); - for (freed = 0; freed < nr_to_scan; freed++) { + while (freed <= nr_to_scan) { struct page *page; mutex_lock(&pool->mutex); @@ -141,6 +141,7 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask, } mutex_unlock(&pool->mutex); ion_page_pool_free_pages(pool, page); + freed += (1 << pool->order); } return freed; diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index da2a63c..36e73c3 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -211,7 +211,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, int nr_to_scan) { struct ion_system_heap *sys_heap; - int nr_total = 0; + int nr_total = 0, nr_freed; int i; sys_heap = container_of(heap, struct ion_system_heap, heap); @@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, for (i = 0; i < num_orders; i++) { struct ion_page_pool *pool = sys_heap->pools[i]; - nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); + nr_total += nr_freed; + /* nr_to_scan can be negative */ + nr_to_scan -= nr_freed; } return nr_total; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] staging: ion: shrink page-pool by page unit 2014-10-23 12:52 ` [RFC 1/2] staging: ion: shrink page-pool by " Gioh Kim @ 2014-10-23 20:36 ` Devendra Naga 2014-10-23 23:16 ` Gioh Kim 0 siblings, 1 reply; 9+ messages in thread From: Devendra Naga @ 2014-10-23 20:36 UTC (permalink / raw) To: Gioh Kim Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, linux-kernel, gunho.lee Hello, On Thu, Oct 23, 2014 at 6:22 PM, Gioh Kim <gioh.kim@lge.com> wrote: > This patch shrink page-pool by page unit. > > Signed-off-by: Gioh Kim <gioh.kim@lge.com> > --- > drivers/staging/android/ion/ion_page_pool.c | 5 +++-- > drivers/staging/android/ion/ion_system_heap.c | 7 +++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > --- cut -- > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index da2a63c..36e73c3 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -211,7 +211,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, > int nr_to_scan) > { > struct ion_system_heap *sys_heap; > - int nr_total = 0; > + int nr_total = 0, nr_freed; > int i; > > sys_heap = container_of(heap, struct ion_system_heap, heap); > @@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, > for (i = 0; i < num_orders; i++) { > struct ion_page_pool *pool = sys_heap->pools[i]; > > - nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); > + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); nr_freed is not initialised to zero. This might result in updating of a garbage value stored initially after nr_freed is created. > + nr_total += nr_freed; > + /* nr_to_scan can be negative */ > + nr_to_scan -= nr_freed; > } > > return nr_total; > -- > 1.7.9.5 > > -- > 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/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] staging: ion: shrink page-pool by page unit 2014-10-23 20:36 ` Devendra Naga @ 2014-10-23 23:16 ` Gioh Kim 2014-10-23 23:31 ` Gioh Kim 0 siblings, 1 reply; 9+ messages in thread From: Gioh Kim @ 2014-10-23 23:16 UTC (permalink / raw) To: Devendra Naga Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, linux-kernel, gunho.lee 2014-10-24 오전 5:36, Devendra Naga 쓴 글: > Hello, > > On Thu, Oct 23, 2014 at 6:22 PM, Gioh Kim <gioh.kim@lge.com> wrote: >> This patch shrink page-pool by page unit. >> >> Signed-off-by: Gioh Kim <gioh.kim@lge.com> >> --- >> drivers/staging/android/ion/ion_page_pool.c | 5 +++-- >> drivers/staging/android/ion/ion_system_heap.c | 7 +++++-- >> 2 files changed, 8 insertions(+), 4 deletions(-) > >> > > --- cut -- > >> diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c >> index da2a63c..36e73c3 100644 >> --- a/drivers/staging/android/ion/ion_system_heap.c >> +++ b/drivers/staging/android/ion/ion_system_heap.c >> @@ -211,7 +211,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >> int nr_to_scan) >> { >> struct ion_system_heap *sys_heap; >> - int nr_total = 0; >> + int nr_total = 0, nr_freed; >> int i; >> >> sys_heap = container_of(heap, struct ion_system_heap, heap); >> @@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >> for (i = 0; i < num_orders; i++) { >> struct ion_page_pool *pool = sys_heap->pools[i]; >> >> - nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); >> + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); > > nr_freed is not initialised to zero. This might result in updating of > a garbage value stored initially after nr_freed is created. Yes, your're right. It's my mistake. I testes this on my platform with custom kernel and copied codes. I missed a line for it. I'm sorry. > >> + nr_total += nr_freed; >> + /* nr_to_scan can be negative */ >> + nr_to_scan -= nr_freed; >> } >> >> return nr_total; >> -- >> 1.7.9.5 >> >> -- >> 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/ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] staging: ion: shrink page-pool by page unit 2014-10-23 23:16 ` Gioh Kim @ 2014-10-23 23:31 ` Gioh Kim 2014-10-23 23:49 ` Devendra Naga 0 siblings, 1 reply; 9+ messages in thread From: Gioh Kim @ 2014-10-23 23:31 UTC (permalink / raw) To: Devendra Naga Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, linux-kernel, gunho.lee 2014-10-24 오전 8:16, Gioh Kim 쓴 글: > > > 2014-10-24 오전 5:36, Devendra Naga 쓴 글: >> Hello, >> >> On Thu, Oct 23, 2014 at 6:22 PM, Gioh Kim <gioh.kim@lge.com> wrote: >>> This patch shrink page-pool by page unit. >>> >>> Signed-off-by: Gioh Kim <gioh.kim@lge.com> >>> --- >>> drivers/staging/android/ion/ion_page_pool.c | 5 +++-- >>> drivers/staging/android/ion/ion_system_heap.c | 7 +++++-- >>> 2 files changed, 8 insertions(+), 4 deletions(-) >> >>> >> >> --- cut -- >> >>> diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c >>> index da2a63c..36e73c3 100644 >>> --- a/drivers/staging/android/ion/ion_system_heap.c >>> +++ b/drivers/staging/android/ion/ion_system_heap.c >>> @@ -211,7 +211,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >>> int nr_to_scan) >>> { >>> struct ion_system_heap *sys_heap; >>> - int nr_total = 0; >>> + int nr_total = 0, nr_freed; >>> int i; >>> >>> sys_heap = container_of(heap, struct ion_system_heap, heap); >>> @@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap *heap, gfp_t gfp_mask, >>> for (i = 0; i < num_orders; i++) { >>> struct ion_page_pool *pool = sys_heap->pools[i]; >>> >>> - nr_total += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); >>> + nr_freed += ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); >> >> nr_freed is not initialised to zero. This might result in updating of >> a garbage value stored initially after nr_freed is created. > > Yes, your're right. It's my mistake. > I testes this on my platform with custom kernel and copied codes. > I missed a line for it. > I'm sorry. Following is the correct code: nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); > >> >>> + nr_total += nr_freed; >>> + /* nr_to_scan can be negative */ >>> + nr_to_scan -= nr_freed; >>> } >>> >>> return nr_total; >>> -- >>> 1.7.9.5 >>> >>> -- >>> 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/ >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 1/2] staging: ion: shrink page-pool by page unit 2014-10-23 23:31 ` Gioh Kim @ 2014-10-23 23:49 ` Devendra Naga 0 siblings, 0 replies; 9+ messages in thread From: Devendra Naga @ 2014-10-23 23:49 UTC (permalink / raw) To: Gioh Kim Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, linux-kernel, gunho.lee On Fri, Oct 24, 2014 at 5:01 AM, Gioh Kim <gioh.kim@lge.com> wrote: > > > 2014-10-24 오전 8:16, Gioh Kim 쓴 글: > >> >> >> 2014-10-24 오전 5:36, Devendra Naga 쓴 글: >>> >>> Hello, >>> >>> On Thu, Oct 23, 2014 at 6:22 PM, Gioh Kim <gioh.kim@lge.com> wrote: >>>> sys_heap = container_of(heap, struct ion_system_heap, heap); >>>> @@ -219,7 +219,10 @@ static int ion_system_heap_shrink(struct ion_heap >>>> *heap, gfp_t gfp_mask, >>>> for (i = 0; i < num_orders; i++) { >>>> struct ion_page_pool *pool = sys_heap->pools[i]; >>>> >>>> - nr_total += ion_page_pool_shrink(pool, gfp_mask, >>>> nr_to_scan); >>>> + nr_freed += ion_page_pool_shrink(pool, gfp_mask, >>>> nr_to_scan); >>> >>> >>> nr_freed is not initialised to zero. This might result in updating of >>> a garbage value stored initially after nr_freed is created. >> >> >> Yes, your're right. It's my mistake. >> I testes this on my platform with custom kernel and copied codes. >> I missed a line for it. >> I'm sorry. > > > Following is the correct code: > nr_freed = ion_page_pool_shrink(pool, gfp_mask, nr_to_scan); > Thanks that is correct. You can send another patch with this fix (it can be patchset v2 or rfc v2). > >> >>> >>>> + nr_total += nr_freed; >>>> + /* nr_to_scan can be negative */ >>>> + nr_to_scan -= nr_freed; >>>> } >>>> >>>> return nr_total; >>>> -- >>>> 1.7.9.5 >>>> >>>> -- >>>> 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/ >>> >>> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 2/2] staging: ion: debugfs to shrink pool 2014-10-23 12:52 [RFC 0/2] enable pool shrinking in page unit Gioh Kim 2014-10-23 12:52 ` [RFC 1/2] staging: ion: shrink page-pool by " Gioh Kim @ 2014-10-23 12:52 ` Gioh Kim 2014-10-27 11:42 ` Dan Carpenter 1 sibling, 1 reply; 9+ messages in thread From: Gioh Kim @ 2014-10-23 12:52 UTC (permalink / raw) To: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin Cc: devel, linux-kernel, gunho.lee, gioh.kim This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink, to shrink pool or get pool size. Reading the file returns pool size and writing occurs to shrink pool. Signed-off-by: Gioh Kim <gioh.kim@lge.com> --- drivers/staging/android/ion/ion.c | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 290d4d2..ecc1121 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1463,43 +1463,29 @@ static const struct file_operations debug_heap_fops = { .release = single_release, }; -#ifdef DEBUG_HEAP_SHRINKER static int debug_shrink_set(void *data, u64 val) { struct ion_heap *heap = data; - struct shrink_control sc; int objs; - sc.gfp_mask = -1; - sc.nr_to_scan = 0; - - if (!val) - return 0; - - objs = heap->shrinker.shrink(&heap->shrinker, &sc); - sc.nr_to_scan = objs; + if (val) + objs = val; + else + objs = heap->ops->shrink(heap, __GFP_HIGHMEM, 0); - heap->shrinker.shrink(&heap->shrinker, &sc); + (void)heap->ops->shrink(heap, __GFP_HIGHMEM, objs); return 0; } static int debug_shrink_get(void *data, u64 *val) { struct ion_heap *heap = data; - struct shrink_control sc; - int objs; - - sc.gfp_mask = -1; - sc.nr_to_scan = 0; - - objs = heap->shrinker.shrink(&heap->shrinker, &sc); - *val = objs; + *val = heap->ops->shrink(heap, __GFP_HIGHMEM, 0); return 0; } DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get, debug_shrink_set, "%llu\n"); -#endif void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap) { @@ -1534,8 +1520,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap) path, heap->name); } -#ifdef DEBUG_HEAP_SHRINKER - if (heap->shrinker.shrink) { + if (heap->ops->shrink) { char debug_name[64]; snprintf(debug_name, 64, "%s_shrink", heap->name); @@ -1550,7 +1535,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap) path, debug_name); } } -#endif + up_write(&dev->lock); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] staging: ion: debugfs to shrink pool 2014-10-23 12:52 ` [RFC 2/2] staging: ion: debugfs to shrink pool Gioh Kim @ 2014-10-27 11:42 ` Dan Carpenter 2014-10-27 12:52 ` Dan Carpenter 0 siblings, 1 reply; 9+ messages in thread From: Dan Carpenter @ 2014-10-27 11:42 UTC (permalink / raw) To: Gioh Kim Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, gunho.lee, linux-kernel On Thu, Oct 23, 2014 at 09:52:20PM +0900, Gioh Kim wrote: > This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink, > to shrink pool or get pool size. > Reading the file returns pool size and writing occurs to shrink pool. The patch looks fine. Of course, the original code was debugfs before it bitrotted and stopped compiling, but is there there a reason why we shouldn't move this to sysfs instead? Just curious. regards, dan carpenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/2] staging: ion: debugfs to shrink pool 2014-10-27 11:42 ` Dan Carpenter @ 2014-10-27 12:52 ` Dan Carpenter 0 siblings, 0 replies; 9+ messages in thread From: Dan Carpenter @ 2014-10-27 12:52 UTC (permalink / raw) To: Gioh Kim Cc: Greg Kroah-Hartman, John Stultz, Rebecca Schultz Zavin, devel, gunho.lee, linux-kernel On Mon, Oct 27, 2014 at 02:42:40PM +0300, Dan Carpenter wrote: > On Thu, Oct 23, 2014 at 09:52:20PM +0900, Gioh Kim wrote: > > This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink, > > to shrink pool or get pool size. > > Reading the file returns pool size and writing occurs to shrink pool. > > The patch looks fine. Of course, the original code was debugfs before > it bitrotted and stopped compiling, but is there there a reason why we > shouldn't move this to sysfs instead? Just curious. > I read the Laura's review comments and it turns out we don't want this enabled by default and debugfs is fine. Question answered. regards, dan carpenter ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-10-27 12:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-23 12:52 [RFC 0/2] enable pool shrinking in page unit Gioh Kim 2014-10-23 12:52 ` [RFC 1/2] staging: ion: shrink page-pool by " Gioh Kim 2014-10-23 20:36 ` Devendra Naga 2014-10-23 23:16 ` Gioh Kim 2014-10-23 23:31 ` Gioh Kim 2014-10-23 23:49 ` Devendra Naga 2014-10-23 12:52 ` [RFC 2/2] staging: ion: debugfs to shrink pool Gioh Kim 2014-10-27 11:42 ` Dan Carpenter 2014-10-27 12:52 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox