Re: [PATCH 1/9] staging: ion: tidy up a bit
On Tue, May 27, 2014 at 11:52 AM, Greg Kroah-Hartman wrote: > On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: >> For aesthetics and readability, rename goto labels, remove >> useless code lines, and clarify function return type. >> >> Signed-off-by: Heesub Shin >> --- >> drivers/staging/android/ion/ion_page_pool.c | 2 +- >> drivers/staging/android/ion/ion_priv.h| 2 +- >> drivers/staging/android/ion/ion_system_heap.c | 57 >> --- >> 3 files changed, 28 insertions(+), 33 deletions(-) > > For this whole series, I need someone from the Android team, or John, to > ack them, as I have no way of testing. Having Colin's ack would be best, but I can do some basic testing when v2 is sent out. thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: > For aesthetics and readability, rename goto labels, remove > useless code lines, and clarify function return type. > > Signed-off-by: Heesub Shin > --- > drivers/staging/android/ion/ion_page_pool.c | 2 +- > drivers/staging/android/ion/ion_priv.h| 2 +- > drivers/staging/android/ion/ion_system_heap.c | 57 > --- > 3 files changed, 28 insertions(+), 33 deletions(-) For this whole series, I need someone from the Android team, or John, to ack them, as I have no way of testing. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Tue, May 27, 2014 at 09:58:18AM +0900, Heesub Shin wrote: > Hello Carpenter, > > On 05/26/2014 07:36 PM, Dan Carpenter wrote: > >On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: > >>@@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct > >>ion_system_heap *heap, > >> > >>info->page = page; > >>info->order = orders[i]; > >>- INIT_LIST_HEAD(>list); > >>return info; > >>} > >>kfree(info); > > > >Wait. How does this code work without that INIT_LIST_HEAD()? What am > >I missing here... > > No problem. As the object info is just a node, not a head, it is > completely useless to initialize it as a list head. > Oh, sorry, you are right. My bad. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Tue, May 27, 2014 at 09:58:18AM +0900, Heesub Shin wrote: Hello Carpenter, On 05/26/2014 07:36 PM, Dan Carpenter wrote: On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: @@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct ion_system_heap *heap, info-page = page; info-order = orders[i]; - INIT_LIST_HEAD(info-list); return info; } kfree(info); Wait. How does this code work without that INIT_LIST_HEAD()? What am I missing here... No problem. As the object info is just a node, not a head, it is completely useless to initialize it as a list head. Oh, sorry, you are right. My bad. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: For aesthetics and readability, rename goto labels, remove useless code lines, and clarify function return type. Signed-off-by: Heesub Shin heesub.s...@samsung.com --- drivers/staging/android/ion/ion_page_pool.c | 2 +- drivers/staging/android/ion/ion_priv.h| 2 +- drivers/staging/android/ion/ion_system_heap.c | 57 --- 3 files changed, 28 insertions(+), 33 deletions(-) For this whole series, I need someone from the Android team, or John, to ack them, as I have no way of testing. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Tue, May 27, 2014 at 11:52 AM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: For aesthetics and readability, rename goto labels, remove useless code lines, and clarify function return type. Signed-off-by: Heesub Shin heesub.s...@samsung.com --- drivers/staging/android/ion/ion_page_pool.c | 2 +- drivers/staging/android/ion/ion_priv.h| 2 +- drivers/staging/android/ion/ion_system_heap.c | 57 --- 3 files changed, 28 insertions(+), 33 deletions(-) For this whole series, I need someone from the Android team, or John, to ack them, as I have no way of testing. Having Colin's ack would be best, but I can do some basic testing when v2 is sent out. thanks -john -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
Hello Carpenter, On 05/26/2014 07:36 PM, Dan Carpenter wrote: On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: @@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct ion_system_heap *heap, info->page = page; info->order = orders[i]; - INIT_LIST_HEAD(>list); return info; } kfree(info); Wait. How does this code work without that INIT_LIST_HEAD()? What am I missing here... No problem. As the object info is just a node, not a head, it is completely useless to initialize it as a list head. regards, Heesub regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: > @@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct > ion_system_heap *heap, > > info->page = page; > info->order = orders[i]; > - INIT_LIST_HEAD(>list); > return info; > } > kfree(info); Wait. How does this code work without that INIT_LIST_HEAD()? What am I missing here... regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] staging: ion: tidy up a bit
For aesthetics and readability, rename goto labels, remove
useless code lines, and clarify function return type.
Signed-off-by: Heesub Shin
---
drivers/staging/android/ion/ion_page_pool.c | 2 +-
drivers/staging/android/ion/ion_priv.h| 2 +-
drivers/staging/android/ion/ion_system_heap.c | 57 ---
3 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/android/ion/ion_page_pool.c
b/drivers/staging/android/ion/ion_page_pool.c
index ecb5fc3..ead4054 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -89,7 +89,7 @@ static struct page *ion_page_pool_remove(struct ion_page_pool
*pool, bool high)
return page;
}
-void *ion_page_pool_alloc(struct ion_page_pool *pool)
+struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
{
struct page *page = NULL;
diff --git a/drivers/staging/android/ion/ion_priv.h
b/drivers/staging/android/ion/ion_priv.h
index 1eba3f2..365c947 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -377,7 +377,7 @@ struct ion_page_pool {
struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order);
void ion_page_pool_destroy(struct ion_page_pool *);
-void *ion_page_pool_alloc(struct ion_page_pool *);
+struct page *ion_page_pool_alloc(struct ion_page_pool *);
void ion_page_pool_free(struct ion_page_pool *, struct page *);
/** ion_page_pool_shrink - shrinks the size of the memory cached in the pool
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index c923633..b554751 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -41,7 +41,7 @@ static int order_to_index(unsigned int order)
return -1;
}
-static unsigned int order_to_size(int order)
+static inline unsigned int order_to_size(int order)
{
return PAGE_SIZE << order;
}
@@ -78,8 +78,6 @@ static struct page *alloc_buffer_page(struct ion_system_heap
*heap,
ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order,
DMA_BIDIRECTIONAL);
}
- if (!page)
- return NULL;
return page;
}
@@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct
ion_system_heap *heap,
info->page = page;
info->order = orders[i];
- INIT_LIST_HEAD(>list);
return info;
}
kfree(info);
@@ -142,7 +139,6 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
heap);
struct sg_table *table;
struct scatterlist *sg;
- int ret;
struct list_head pages;
struct page_info *info, *tmp_info;
int i = 0;
@@ -160,24 +156,23 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
info = alloc_largest_available(sys_heap, buffer, size_remaining,
max_order);
if (!info)
- goto err;
+ goto free_pages;
list_add_tail(>list, );
- size_remaining -= (1 << info->order) * PAGE_SIZE;
+ size_remaining -= PAGE_SIZE << info->order;
max_order = info->order;
i++;
}
table = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!table)
- goto err;
+ goto free_pages;
- ret = sg_alloc_table(table, i, GFP_KERNEL);
- if (ret)
- goto err1;
+ if (sg_alloc_table(table, i, GFP_KERNEL))
+ goto free_table;
sg = table->sgl;
list_for_each_entry_safe(info, tmp_info, , list) {
struct page *page = info->page;
- sg_set_page(sg, page, (1 << info->order) * PAGE_SIZE, 0);
+ sg_set_page(sg, page, PAGE_SIZE << info->order, 0);
sg = sg_next(sg);
list_del(>list);
kfree(info);
@@ -185,9 +180,10 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
buffer->priv_virt = table;
return 0;
-err1:
+
+free_table:
kfree(table);
-err:
+free_pages:
list_for_each_entry_safe(info, tmp_info, , list) {
free_buffer_page(sys_heap, buffer, info->page, info->order);
kfree(info);
@@ -197,14 +193,12 @@ err:
static void ion_system_heap_free(struct ion_buffer *buffer)
{
- struct ion_heap *heap = buffer->heap;
- struct ion_system_heap *sys_heap = container_of(heap,
+ struct ion_system_heap *sys_heap = container_of(buffer->heap,
struct ion_system_heap,
heap);
struct sg_table *table =
[PATCH 1/9] staging: ion: tidy up a bit
For aesthetics and readability, rename goto labels, remove
useless code lines, and clarify function return type.
Signed-off-by: Heesub Shin heesub.s...@samsung.com
---
drivers/staging/android/ion/ion_page_pool.c | 2 +-
drivers/staging/android/ion/ion_priv.h| 2 +-
drivers/staging/android/ion/ion_system_heap.c | 57 ---
3 files changed, 28 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/android/ion/ion_page_pool.c
b/drivers/staging/android/ion/ion_page_pool.c
index ecb5fc3..ead4054 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -89,7 +89,7 @@ static struct page *ion_page_pool_remove(struct ion_page_pool
*pool, bool high)
return page;
}
-void *ion_page_pool_alloc(struct ion_page_pool *pool)
+struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
{
struct page *page = NULL;
diff --git a/drivers/staging/android/ion/ion_priv.h
b/drivers/staging/android/ion/ion_priv.h
index 1eba3f2..365c947 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -377,7 +377,7 @@ struct ion_page_pool {
struct ion_page_pool *ion_page_pool_create(gfp_t gfp_mask, unsigned int order);
void ion_page_pool_destroy(struct ion_page_pool *);
-void *ion_page_pool_alloc(struct ion_page_pool *);
+struct page *ion_page_pool_alloc(struct ion_page_pool *);
void ion_page_pool_free(struct ion_page_pool *, struct page *);
/** ion_page_pool_shrink - shrinks the size of the memory cached in the pool
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index c923633..b554751 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -41,7 +41,7 @@ static int order_to_index(unsigned int order)
return -1;
}
-static unsigned int order_to_size(int order)
+static inline unsigned int order_to_size(int order)
{
return PAGE_SIZE order;
}
@@ -78,8 +78,6 @@ static struct page *alloc_buffer_page(struct ion_system_heap
*heap,
ion_pages_sync_for_device(NULL, page, PAGE_SIZE order,
DMA_BIDIRECTIONAL);
}
- if (!page)
- return NULL;
return page;
}
@@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct
ion_system_heap *heap,
info-page = page;
info-order = orders[i];
- INIT_LIST_HEAD(info-list);
return info;
}
kfree(info);
@@ -142,7 +139,6 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
heap);
struct sg_table *table;
struct scatterlist *sg;
- int ret;
struct list_head pages;
struct page_info *info, *tmp_info;
int i = 0;
@@ -160,24 +156,23 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
info = alloc_largest_available(sys_heap, buffer, size_remaining,
max_order);
if (!info)
- goto err;
+ goto free_pages;
list_add_tail(info-list, pages);
- size_remaining -= (1 info-order) * PAGE_SIZE;
+ size_remaining -= PAGE_SIZE info-order;
max_order = info-order;
i++;
}
table = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!table)
- goto err;
+ goto free_pages;
- ret = sg_alloc_table(table, i, GFP_KERNEL);
- if (ret)
- goto err1;
+ if (sg_alloc_table(table, i, GFP_KERNEL))
+ goto free_table;
sg = table-sgl;
list_for_each_entry_safe(info, tmp_info, pages, list) {
struct page *page = info-page;
- sg_set_page(sg, page, (1 info-order) * PAGE_SIZE, 0);
+ sg_set_page(sg, page, PAGE_SIZE info-order, 0);
sg = sg_next(sg);
list_del(info-list);
kfree(info);
@@ -185,9 +180,10 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
buffer-priv_virt = table;
return 0;
-err1:
+
+free_table:
kfree(table);
-err:
+free_pages:
list_for_each_entry_safe(info, tmp_info, pages, list) {
free_buffer_page(sys_heap, buffer, info-page, info-order);
kfree(info);
@@ -197,14 +193,12 @@ err:
static void ion_system_heap_free(struct ion_buffer *buffer)
{
- struct ion_heap *heap = buffer-heap;
- struct ion_system_heap *sys_heap = container_of(heap,
+ struct ion_system_heap *sys_heap = container_of(buffer-heap,
struct ion_system_heap,
heap);
Re: [PATCH 1/9] staging: ion: tidy up a bit
On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: @@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct ion_system_heap *heap, info-page = page; info-order = orders[i]; - INIT_LIST_HEAD(info-list); return info; } kfree(info); Wait. How does this code work without that INIT_LIST_HEAD()? What am I missing here... regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/9] staging: ion: tidy up a bit
Hello Carpenter, On 05/26/2014 07:36 PM, Dan Carpenter wrote: On Mon, May 26, 2014 at 07:04:53PM +0900, Heesub Shin wrote: @@ -124,7 +122,6 @@ static struct page_info *alloc_largest_available(struct ion_system_heap *heap, info-page = page; info-order = orders[i]; - INIT_LIST_HEAD(info-list); return info; } kfree(info); Wait. How does this code work without that INIT_LIST_HEAD()? What am I missing here... No problem. As the object info is just a node, not a head, it is completely useless to initialize it as a list head. regards, Heesub regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
