* [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON
@ 2020-08-16 18:40 Tomer Samara
2020-08-16 19:23 ` [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON Tomer Samara
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Tomer Samara @ 2020-08-16 18:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Riley Andrews, Laura Abbott,
Sumit Semwal, Todd Kjos, Martijn Coenen, Joel Fernandes,
Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, devel,
dri-devel, linux-kernel
This series convert BUGs/BUG_ONs to WARNs/WARN_ONs
Tomer Samara (4):
staging: android: Replace BUG_ON with WARN_ON
staging: android: Add error handling to ion_page_pool_shrink
staging: android: Convert BUG to WARN
staging: android: Add error handling to order_to_index callers
drivers/staging/android/ion/ion_page_pool.c | 14 ++++++++++----
drivers/staging/android/ion/ion_system_heap.c | 16 +++++++++++++---
2 files changed, 23 insertions(+), 7 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
2020-08-16 18:40 [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON Tomer Samara
@ 2020-08-16 19:23 ` Tomer Samara
2020-08-18 14:11 ` Greg Kroah-Hartman
2020-08-16 19:24 ` [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink Tomer Samara
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Tomer Samara @ 2020-08-16 19:23 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Riley Andrews, Laura Abbott,
Sumit Semwal, Todd Kjos, Martijn Coenen, Joel Fernandes,
Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, devel,
dri-devel, linux-kernel
BUG_ON() is replaced with WARN_ON at ion_page_pool.c
Fixes the following issue:
Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan BUG() or BUG_ON().
Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
---
drivers/staging/android/ion/ion_page_pool.c | 12 ++++++++----
1 file 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 0198b886d906..c1b9eda35c96 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -46,11 +46,13 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high)
struct page *page;
if (high) {
- BUG_ON(!pool->high_count);
+ if (WARN_ON(!pool->high_count))
+ return NULL;
page = list_first_entry(&pool->high_items, struct page, lru);
pool->high_count--;
} else {
- BUG_ON(!pool->low_count);
+ if (WARN_ON(!pool->low_count))
+ return NULL;
page = list_first_entry(&pool->low_items, struct page, lru);
pool->low_count--;
}
@@ -65,7 +67,8 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
{
struct page *page = NULL;
- BUG_ON(!pool);
+ if (WARN_ON(!pool))
+ return NULL;
mutex_lock(&pool->mutex);
if (pool->high_count)
@@ -82,7 +85,8 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
void ion_page_pool_free(struct ion_page_pool *pool, struct page *page)
{
- BUG_ON(pool->order != compound_order(page));
+ if (WARN_ON(pool->order != compound_order(page)))
+ return;
ion_page_pool_add(pool, page);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink
2020-08-16 18:40 [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON Tomer Samara
2020-08-16 19:23 ` [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON Tomer Samara
@ 2020-08-16 19:24 ` Tomer Samara
2020-08-18 14:10 ` Greg Kroah-Hartman
2020-08-16 19:30 ` [PATCH v2 3/4] staging: android: Convert BUG to WARN Tomer Samara
2020-08-16 19:31 ` [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers Tomer Samara
3 siblings, 1 reply; 11+ messages in thread
From: Tomer Samara @ 2020-08-16 19:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Riley Andrews, Laura Abbott,
Sumit Semwal, Todd Kjos, Martijn Coenen, Joel Fernandes,
Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, devel,
dri-devel, linux-kernel
Add error check to ion_page_pool_shrink after calling
ion_page_pool_remove, due to converting BUG_ON to WARN_ON.
Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
---
drivers/staging/android/ion/ion_page_pool.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index c1b9eda35c96..031550473000 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -128,6 +128,8 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
break;
}
mutex_unlock(&pool->mutex);
+ if (!page)
+ break;
ion_page_pool_free_pages(pool, page);
freed += (1 << pool->order);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] staging: android: Convert BUG to WARN
2020-08-16 18:40 [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON Tomer Samara
2020-08-16 19:23 ` [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON Tomer Samara
2020-08-16 19:24 ` [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink Tomer Samara
@ 2020-08-16 19:30 ` Tomer Samara
2020-08-18 14:11 ` Greg Kroah-Hartman
2020-08-16 19:31 ` [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers Tomer Samara
3 siblings, 1 reply; 11+ messages in thread
From: Tomer Samara @ 2020-08-16 19:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Riley Andrews, Laura Abbott,
Sumit Semwal, Todd Kjos, Martijn Coenen, Joel Fernandes,
Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, devel,
dri-devel, linux-kernel
replace BUG() with WARN() at ion_sytem_heap.c, this
fix the following checkpatch issue:
Avoid crashing the kernel - try using WARN_ON &
recovery code ratherthan BUG() or BUG_ON().
Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
---
drivers/staging/android/ion/ion_system_heap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index eac0632ab4e8..37065a59ca69 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -30,7 +30,8 @@ static int order_to_index(unsigned int order)
for (i = 0; i < NUM_ORDERS; i++)
if (order == orders[i])
return i;
- BUG();
+
+ WARN(1, "%s: Did not found index to order %d", __FUNCTION__, order);
return -1;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers
2020-08-16 18:40 [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON Tomer Samara
` (2 preceding siblings ...)
2020-08-16 19:30 ` [PATCH v2 3/4] staging: android: Convert BUG to WARN Tomer Samara
@ 2020-08-16 19:31 ` Tomer Samara
2020-08-18 14:12 ` Greg Kroah-Hartman
3 siblings, 1 reply; 11+ messages in thread
From: Tomer Samara @ 2020-08-16 19:31 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Arve Hjønnevåg, Riley Andrews, Laura Abbott,
Sumit Semwal, Todd Kjos, Martijn Coenen, Joel Fernandes,
Christian Brauner, Hridya Valsaraju, Suren Baghdasaryan, devel,
dri-devel, linux-kernel
Add error check to:
- free_buffer_page
- alloc_buffer_page
after calling order_to_index, due to converting BUG to WARN at
order_to_index.
Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
---
drivers/staging/android/ion/ion_system_heap.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
index 37065a59ca69..1e73bfc88884 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -49,8 +49,13 @@ static struct page *alloc_buffer_page(struct ion_system_heap *heap,
struct ion_buffer *buffer,
unsigned long order)
{
- struct ion_page_pool *pool = heap->pools[order_to_index(order)];
+ struct ion_page_pool *pool;
+ int index = order_to_index(order);
+
+ if (index < 0)
+ return NULL;
+ pool = heap->pools[index];
return ion_page_pool_alloc(pool);
}
@@ -59,6 +64,7 @@ static void free_buffer_page(struct ion_system_heap *heap,
{
struct ion_page_pool *pool;
unsigned int order = compound_order(page);
+ int index;
/* go to system */
if (buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE) {
@@ -66,8 +72,11 @@ static void free_buffer_page(struct ion_system_heap *heap,
return;
}
- pool = heap->pools[order_to_index(order)];
+ index = order_to_index(order);
+ if (index < 0)
+ return;
+ pool = heap->pools[index];
ion_page_pool_free(pool, page);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink
2020-08-16 19:24 ` [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink Tomer Samara
@ 2020-08-18 14:10 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-18 14:10 UTC (permalink / raw)
To: Tomer Samara
Cc: devel, Todd Kjos, Suren Baghdasaryan, Riley Andrews, dri-devel,
linux-kernel, Hridya Valsaraju, Arve Hjønnevåg,
Joel Fernandes, Laura Abbott, Martijn Coenen, Sumit Semwal,
Christian Brauner
On Sun, Aug 16, 2020 at 10:24:22PM +0300, Tomer Samara wrote:
> Add error check to ion_page_pool_shrink after calling
> ion_page_pool_remove, due to converting BUG_ON to WARN_ON.
>
> Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
So this fixes a previous patch? That's not good, please merge them
together so you do not cause a bug and then fix it up later on.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
2020-08-16 19:23 ` [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON Tomer Samara
@ 2020-08-18 14:11 ` Greg Kroah-Hartman
2020-08-18 14:19 ` Tomer Samara
0 siblings, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-18 14:11 UTC (permalink / raw)
To: Tomer Samara
Cc: devel, Todd Kjos, Suren Baghdasaryan, Riley Andrews, dri-devel,
linux-kernel, Hridya Valsaraju, Arve Hjønnevåg,
Joel Fernandes, Laura Abbott, Martijn Coenen, Sumit Semwal,
Christian Brauner
On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote:
> BUG_ON() is replaced with WARN_ON at ion_page_pool.c
Why?
> Fixes the following issue:
> Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan BUG() or BUG_ON().
Ideally you can get rid of WARN_ON() too, right?
Many systems run in panic-on-warn mode, so this really does not change
anything. Try fixing this up properly to not crash at all.
>
> Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
> ---
> drivers/staging/android/ion/ion_page_pool.c | 12 ++++++++----
> 1 file 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 0198b886d906..c1b9eda35c96 100644
> --- a/drivers/staging/android/ion/ion_page_pool.c
> +++ b/drivers/staging/android/ion/ion_page_pool.c
> @@ -46,11 +46,13 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high)
> struct page *page;
>
> if (high) {
> - BUG_ON(!pool->high_count);
> + if (WARN_ON(!pool->high_count))
> + return NULL;
And can you test this that it works properly?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] staging: android: Convert BUG to WARN
2020-08-16 19:30 ` [PATCH v2 3/4] staging: android: Convert BUG to WARN Tomer Samara
@ 2020-08-18 14:11 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-18 14:11 UTC (permalink / raw)
To: Tomer Samara
Cc: devel, Todd Kjos, Suren Baghdasaryan, Riley Andrews, dri-devel,
linux-kernel, Hridya Valsaraju, Arve Hjønnevåg,
Joel Fernandes, Laura Abbott, Martijn Coenen, Sumit Semwal,
Christian Brauner
On Sun, Aug 16, 2020 at 10:30:10PM +0300, Tomer Samara wrote:
> replace BUG() with WARN() at ion_sytem_heap.c, this
> fix the following checkpatch issue:
> Avoid crashing the kernel - try using WARN_ON &
> recovery code ratherthan BUG() or BUG_ON().
>
> Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
> ---
> drivers/staging/android/ion/ion_system_heap.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c
> index eac0632ab4e8..37065a59ca69 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -30,7 +30,8 @@ static int order_to_index(unsigned int order)
> for (i = 0; i < NUM_ORDERS; i++)
> if (order == orders[i])
> return i;
> - BUG();
> +
> + WARN(1, "%s: Did not found index to order %d", __FUNCTION__, order);
Same question as before, I think this didn't really change anything :(
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers
2020-08-16 19:31 ` [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers Tomer Samara
@ 2020-08-18 14:12 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-18 14:12 UTC (permalink / raw)
To: Tomer Samara
Cc: devel, Todd Kjos, Suren Baghdasaryan, Riley Andrews, dri-devel,
linux-kernel, Hridya Valsaraju, Arve Hjønnevåg,
Joel Fernandes, Laura Abbott, Martijn Coenen, Sumit Semwal,
Christian Brauner
On Sun, Aug 16, 2020 at 10:31:22PM +0300, Tomer Samara wrote:
> Add error check to:
> - free_buffer_page
> - alloc_buffer_page
> after calling order_to_index, due to converting BUG to WARN at
> order_to_index.
You are fixing a bug you caused in a previous patch, not good :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
2020-08-18 14:11 ` Greg Kroah-Hartman
@ 2020-08-18 14:19 ` Tomer Samara
2020-08-18 14:27 ` Greg Kroah-Hartman
0 siblings, 1 reply; 11+ messages in thread
From: Tomer Samara @ 2020-08-18 14:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devel, Todd Kjos, Suren Baghdasaryan, Riley Andrews, dri-devel,
linux-kernel, Hridya Valsaraju, Arve Hjønnevåg,
Joel Fernandes, Laura Abbott, Martijn Coenen, Sumit Semwal,
Christian Brauner
On Tue, Aug 18, 2020 at 04:11:06PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote:
> > BUG_ON() is replaced with WARN_ON at ion_page_pool.c
>
> Why?
>
> > Fixes the following issue:
> > Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan BUG() or BUG_ON().
>
> Ideally you can get rid of WARN_ON() too, right?
>
> Many systems run in panic-on-warn mode, so this really does not change
> anything. Try fixing this up properly to not crash at all.
>
You mean by that to just remove the WARN_ON and leave the condition the
same?
> >
> > Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
> > ---
> > drivers/staging/android/ion/ion_page_pool.c | 12 ++++++++----
> > 1 file 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 0198b886d906..c1b9eda35c96 100644
> > --- a/drivers/staging/android/ion/ion_page_pool.c
> > +++ b/drivers/staging/android/ion/ion_page_pool.c
> > @@ -46,11 +46,13 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high)
> > struct page *page;
> >
> > if (high) {
> > - BUG_ON(!pool->high_count);
> > + if (WARN_ON(!pool->high_count))
> > + return NULL;
>
> And can you test this that it works properly?
>
> thanks,
>
> greg k-h
Will do :)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON
2020-08-18 14:19 ` Tomer Samara
@ 2020-08-18 14:27 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-18 14:27 UTC (permalink / raw)
To: Tomer Samara
Cc: devel, Todd Kjos, Martijn Coenen, linux-kernel, dri-devel,
Joel Fernandes, Riley Andrews, Arve Hjønnevåg,
Hridya Valsaraju, Laura Abbott, Suren Baghdasaryan, Sumit Semwal,
Christian Brauner
On Tue, Aug 18, 2020 at 05:19:40PM +0300, Tomer Samara wrote:
> On Tue, Aug 18, 2020 at 04:11:06PM +0200, Greg Kroah-Hartman wrote:
> > On Sun, Aug 16, 2020 at 10:23:25PM +0300, Tomer Samara wrote:
> > > BUG_ON() is replaced with WARN_ON at ion_page_pool.c
> >
> > Why?
> >
> > > Fixes the following issue:
> > > Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan BUG() or BUG_ON().
> >
> > Ideally you can get rid of WARN_ON() too, right?
> >
> > Many systems run in panic-on-warn mode, so this really does not change
> > anything. Try fixing this up properly to not crash at all.
> >
> You mean by that to just remove the WARN_ON and leave the condition the
> same?
Or fix the problem that could ever cause this check to fire.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-08-18 14:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-16 18:40 [PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON Tomer Samara
2020-08-16 19:23 ` [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON Tomer Samara
2020-08-18 14:11 ` Greg Kroah-Hartman
2020-08-18 14:19 ` Tomer Samara
2020-08-18 14:27 ` Greg Kroah-Hartman
2020-08-16 19:24 ` [PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink Tomer Samara
2020-08-18 14:10 ` Greg Kroah-Hartman
2020-08-16 19:30 ` [PATCH v2 3/4] staging: android: Convert BUG to WARN Tomer Samara
2020-08-18 14:11 ` Greg Kroah-Hartman
2020-08-16 19:31 ` [PATCH v2 4/4] staging: android: Add error handling to order_to_index callers Tomer Samara
2020-08-18 14:12 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).