[PATCH] staging: netlogic: clear alignment style issues

[Tomer Samara]

Clear checkpatch alignment style issues in xlr_net.c.
CHECK: Alignment should match open parenthesis

Signed-off-by: Tomer Samara 
---
 drivers/staging/netlogic/xlr_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/netlogic/xlr_net.c 
b/drivers/staging/netlogic/xlr_net.c
index 204fcdfc022f..69ea61faf8fa 100644
--- a/drivers/staging/netlogic/xlr_net.c
+++ b/drivers/staging/netlogic/xlr_net.c
@@ -355,7 +355,7 @@ static void xlr_stats(struct net_device *ndev, struct 
rtnl_link_stats64 *stats)
stats->rx_missed_errors);
 
stats->tx_aborted_errors = xlr_nae_rdreg(priv->base_addr,
-   TX_EXCESSIVE_COLLISION_PACKET_COUNTER);
+
TX_EXCESSIVE_COLLISION_PACKET_COUNTER);
stats->tx_carrier_errors = xlr_nae_rdreg(priv->base_addr,
 TX_DROP_FRAME_COUNTER);
stats->tx_fifo_errors = xlr_nae_rdreg(priv->base_addr,
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: rts5208: clear alignment style issues

[Tomer Samara]

  Clear checkpatch alignment style issues in rtsx_transport.c.
  CHECK: Alignment should match open parenthesis

Signed-off-by: Tomer Samara 
---
 drivers/staging/rts5208/rtsx_transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rts5208/rtsx_transport.c 
b/drivers/staging/rts5208/rtsx_transport.c
index 5f1eefe80f1e..0027bcf638ad 100644
--- a/drivers/staging/rts5208/rtsx_transport.c
+++ b/drivers/staging/rts5208/rtsx_transport.c
@@ -678,7 +678,7 @@ static int rtsx_transfer_buf(struct rtsx_chip *chip, u8 
card, void *buf,
 
/* Wait for TRANS_OK_INT */
timeleft = wait_for_completion_interruptible_timeout(&trans_done,
-   msecs_to_jiffies(timeout));
+
msecs_to_jiffies(timeout));
if (timeleft <= 0) {
dev_dbg(rtsx_dev(chip), "Timeout (%s %d)\n",
__func__, __LINE__);
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wfx: clear alignment style issues

[Tomer Samara]

  Clear checkpatch alignment style issues in debug.c.
  CHECK: Alignment should match open parenthesis

Signed-off-by: Tomer Samara 
---
 drivers/staging/wfx/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 3f1712b7c919..83ccbab50899 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -32,7 +32,7 @@ static const struct trace_print_flags wfx_reg_print_map[] = {
 };
 
 static const char *get_symbol(unsigned long val,
-   const struct trace_print_flags *symbol_array)
+ const struct trace_print_flags *symbol_array)
 {
int i;
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c

[Tomer Samara]

Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
which works as follow:
wfx_full_send() - simple wrapper for both wfx_fill_header()
  and wfx_cmd_send().
wfx_full_send_no_reply_async() - wrapper for both but with
 NULL as reply and size zero.
wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
   but with false async value
wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
but also free the struct hif_msg.

Signed-off-by: Tomer Samara 
---
 drivers/staging/wfx/hif_tx.c | 179 ---
 1 file changed, 79 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 5110f9b93762..1ee84e5d47ef 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -40,7 +40,7 @@ static void wfx_fill_header(struct hif_msg *hif, int if_id,
 
 static void *wfx_alloc_hif(size_t body_len, struct hif_msg **hif)
 {
-   *hif = kzalloc(sizeof(struct hif_msg) + body_len, GFP_KERNEL);
+   *hif = kzalloc(sizeof(*hif) + body_len, GFP_KERNEL);
if (*hif)
return (*hif)->body;
else
@@ -123,9 +123,38 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg 
*request,
return ret;
 }
 
+int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void *reply, 
size_t reply_len,
+ bool async, int if_id, unsigned int cmd, int size)
+{
+   wfx_fill_header(hif, if_id, cmd, size);
+   return wfx_cmd_send(wdev, hif, reply, reply_len, async);
+}
+
+int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct hif_msg *hif, 
int if_id,
+unsigned int cmd, int size, bool async)
+{
+   return wfx_full_send(wdev, hif, NULL, 0, async, if_id, cmd, size);
+}
+
+int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg *hif, int 
if_id,
+  unsigned int cmd, int size)
+{
+   return wfx_full_send_no_reply_async(wdev, hif, if_id, cmd, size, false);
+}
+
+int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct hif_msg *hif, int 
if_id,
+   unsigned int cmd, int size)
+{
+   int ret;
+
+   ret = wfx_full_send_no_reply(wdev, hif, if_id, cmd, size);
+   kfree(hif);
+   return ret;
+}
+
 // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to 
any
 // request anymore. We need to slightly hack struct wfx_hif_cmd for that job. 
Be
-// carefull to only call this funcion during device unregister.
+// careful to only call this function during device unregister.
 int hif_shutdown(struct wfx_dev *wdev)
 {
int ret;
@@ -136,8 +165,8 @@ int hif_shutdown(struct wfx_dev *wdev)
wfx_alloc_hif(0, &hif);
if (!hif)
return -ENOMEM;
-   wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
-   ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
+   ret = wfx_full_send_no_reply_async(wdev, hif, -1, HIF_REQ_ID_SHUT_DOWN,
+  0, true);
// After this command, chip won't reply. Be sure to give enough time to
// bh to send buffer:
msleep(100);
@@ -154,7 +183,6 @@ int hif_shutdown(struct wfx_dev *wdev)
 
 int hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len)
 {
-   int ret;
size_t buf_len = sizeof(struct hif_req_configuration) + len;
struct hif_msg *hif;
struct hif_req_configuration *body = wfx_alloc_hif(buf_len, &hif);
@@ -163,25 +191,20 @@ int hif_configuration(struct wfx_dev *wdev, const u8 
*conf, size_t len)
return -ENOMEM;
body->length = cpu_to_le16(len);
memcpy(body->pds_data, conf, len);
-   wfx_fill_header(hif, -1, HIF_REQ_ID_CONFIGURATION, buf_len);
-   ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-   kfree(hif);
-   return ret;
+   return wfx_full_send_no_reply_free(wdev, hif, -1, 
HIF_REQ_ID_CONFIGURATION,
+  buf_len);
 }
 
 int hif_reset(struct wfx_vif *wvif, bool reset_stat)
 {
-   int ret;
struct hif_msg *hif;
struct hif_req_reset *body = wfx_alloc_hif(sizeof(*body), &hif);
 
if (!hif)
return -ENOMEM;
body->reset_flags.reset_stat = reset_stat;
-   wfx_fill_header(hif, wvif->id, HIF_REQ_ID_RESET, sizeof(*body));
-   ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
-   kfree(hif);
-   return ret;
+   return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+  HIF_REQ_ID_RESET, sizeof(*body));
 }
 
 int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
@@ -198,9 +221,8 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 
mib_id,
goto out

Re: [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c

[Tomer Samara]

On Wed, Aug 05, 2020 at 11:04:25AM +0200, Greg KH wrote:
> On Wed, Aug 05, 2020 at 11:56:08AM +0300, Tomer Samara wrote:
> > Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
> > wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
> > which works as follow:
> > wfx_full_send() - simple wrapper for both wfx_fill_header()
> >   and wfx_cmd_send().
> > wfx_full_send_no_reply_async() - wrapper for both but with
> >  NULL as reply and size zero.
> > wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
> >but with false async value
> > wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
> > but also free the struct hif_msg.
> 
> Please only do one-thing-per-patch.  Why shouldn't this be a 4 patch
> series?
> 
> thanks,
> 
> greg k-h

All of the 4 functions are wrappers for the same duplicate code when 
every time there are different flags, so they are all connected, it is
feel to me more legit to patch them all together, should I split them
into 4 different patches?

Thanks,
Tomer Samara
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: wfx: refactor to avoid duplication at hif_tx.c

[Tomer Samara]

Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
which works as follow:
wfx_full_send() - simple wrapper for both wfx_fill_header()
  and wfx_cmd_send().
wfx_full_send_no_reply_async() - wrapper for both but with
 NULL as reply and size zero.
wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
   but with false async value
wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
but also free the struct hif_msg.

Signed-off-by: Tomer Samara 
---
Changes in v2:
 - Changed these functions to static

drivers/staging/wfx/hif_tx.c | 180 ---
 1 file changed, 80 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 5110f9b93762..17f668fa40a0 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -40,7 +40,7 @@ static void wfx_fill_header(struct hif_msg *hif, int if_id,
 
 static void *wfx_alloc_hif(size_t body_len, struct hif_msg **hif)
 {
-   *hif = kzalloc(sizeof(struct hif_msg) + body_len, GFP_KERNEL);
+   *hif = kzalloc(sizeof(*hif) + body_len, GFP_KERNEL);
if (*hif)
return (*hif)->body;
else
@@ -123,9 +123,39 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg 
*request,
return ret;
 }
 
+static int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void 
*reply,
+size_t reply_len, bool async, int if_id, unsigned int 
cmd,
+int size)
+{
+   wfx_fill_header(hif, if_id, cmd, size);
+   return wfx_cmd_send(wdev, hif, reply, reply_len, async);
+}
+
+static int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct hif_msg 
*hif, int if_id,
+   unsigned int cmd, int size, bool async)
+{
+   return wfx_full_send(wdev, hif, NULL, 0, async, if_id, cmd, size);
+}
+
+static int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg *hif, 
int if_id,
+ unsigned int cmd, int size)
+{
+   return wfx_full_send_no_reply_async(wdev, hif, if_id, cmd, size, false);
+}
+
+static int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct hif_msg 
*hif, int if_id,
+  unsigned int cmd, int size)
+{
+   int ret;
+
+   ret = wfx_full_send_no_reply(wdev, hif, if_id, cmd, size);
+   kfree(hif);
+   return ret;
+}
+
 // This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to 
any
 // request anymore. We need to slightly hack struct wfx_hif_cmd for that job. 
Be
-// carefull to only call this funcion during device unregister.
+// careful to only call this function during device unregister.
 int hif_shutdown(struct wfx_dev *wdev)
 {
int ret;
@@ -136,8 +166,8 @@ int hif_shutdown(struct wfx_dev *wdev)
wfx_alloc_hif(0, &hif);
if (!hif)
return -ENOMEM;
-   wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
-   ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
+   ret = wfx_full_send_no_reply_async(wdev, hif, -1, HIF_REQ_ID_SHUT_DOWN,
+  0, true);
// After this command, chip won't reply. Be sure to give enough time to
// bh to send buffer:
msleep(100);
@@ -154,7 +184,6 @@ int hif_shutdown(struct wfx_dev *wdev)
 
 int hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len)
 {
-   int ret;
size_t buf_len = sizeof(struct hif_req_configuration) + len;
struct hif_msg *hif;
struct hif_req_configuration *body = wfx_alloc_hif(buf_len, &hif);
@@ -163,25 +192,20 @@ int hif_configuration(struct wfx_dev *wdev, const u8 
*conf, size_t len)
return -ENOMEM;
body->length = cpu_to_le16(len);
memcpy(body->pds_data, conf, len);
-   wfx_fill_header(hif, -1, HIF_REQ_ID_CONFIGURATION, buf_len);
-   ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
-   kfree(hif);
-   return ret;
+   return wfx_full_send_no_reply_free(wdev, hif, -1, 
HIF_REQ_ID_CONFIGURATION,
+  buf_len);
 }
 
 int hif_reset(struct wfx_vif *wvif, bool reset_stat)
 {
-   int ret;
struct hif_msg *hif;
struct hif_req_reset *body = wfx_alloc_hif(sizeof(*body), &hif);
 
if (!hif)
return -ENOMEM;
body->reset_flags.reset_stat = reset_stat;
-   wfx_fill_header(hif, wvif->id, HIF_REQ_ID_RESET, sizeof(*body));
-   ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
-   kfree(hif);
-   return ret;
+   return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+  HIF_REQ_ID_RESET, sizeof(*body));
 }
 
 int hif_read_mib(struct wfx_dev *

[PATCH 0/3] Replac BAG/BAG_ON with WARN/WARN_ON

[Tomer Samara]

This series convert BUGs/BUG_ONs to WARNs/WARN_ONs

Tomer Samara (3):
  staging: androind: Replace BUG_ONs with WARN_ONs
  staging: androind: Add error handling to ion_page_pool_shrink
  staging: androind: Convert BUG() to WARN()

 drivers/staging/android/ion/ion_page_pool.c   | 14 ++
 drivers/staging/android/ion/ion_system_heap.c |  3 ++-
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: androind: Add error handling to ion_page_pool_shrink

[Tomer Samara]

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 
---
 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 4acc0eebf781..c61548ecf7f2 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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: androind: Convert BUG() to WARN()

[Tomer Samara]

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: androind: Replace BUG_ONs with WARN_ONs

[Tomer Samara]

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 
---
 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..4acc0eebf781 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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: androind: Convert BUG() to WARN()

[Tomer Samara]

On Sun, Aug 16, 2020 at 10:34:50AM -0700, Randy Dunlap wrote:
> On 8/16/20 10:22 AM, 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 
> > ---
> >  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;
> >  }
> 
> Hi,
> Did you look at what happens when order_to_index() returns -1
> to its callers?
> 
> 
> Also: fix spelling in Subjects: android and BUG/BUG_ON
> 
> -- 
> ~Randy
> 

Hi,
yes I've made a patch for that too but it seems I forgot to
include him in the patch set.
I will send new version for this patch set soon.

Thanks
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/4] Replace BUG/BUG_ON to WARN/WARN_ON

[Tomer Samara]

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON

[Tomer Samara]

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/4] staging: android: Add error handling to ion_page_pool_shrink

[Tomer Samara]

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/4] staging: android: Convert BUG to WARN

[Tomer Samara]

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/4] staging: android: Add error handling to order_to_index callers

[Tomer Samara]

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 
---
 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..1e73bfc4 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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] *** SUBJECT HERE ***

[Tomer Samara]

*** BLURB HERE ***

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

/tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch
/tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch
/tmp/0003-staging-android-Convert-BUG-to-WARN.patch
/tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] *** SUBJECT HERE ***

[Tomer Samara]

*** BLURB HERE ***

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

/tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch
/tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch
/tmp/0003-staging-android-Convert-BUG-to-WARN.patch
/tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] *** SUBJECT HERE ***

[Tomer Samara]

*** BLURB HERE ***

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

/tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch
/tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch
/tmp/0003-staging-android-Convert-BUG-to-WARN.patch
/tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/4] *** SUBJECT HERE ***

[Tomer Samara]

*** BLURB HERE ***

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

/tmp/0001-staging-android-Replace-BUG_ON-with-WARN_ON.patch
/tmp/0002-staging-android-Add-error-handling-to-ion_page_pool_.patch
/tmp/0003-staging-android-Convert-BUG-to-WARN.patch
/tmp/0004-staging-android-Add-error-handling-to-order_to_index.patch
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/4] *** SUBJECT HERE ***

[Tomer Samara]

On Tue, Aug 18, 2020 at 11:50:35AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 18, 2020 at 12:17:08PM +0300, Tomer Samara wrote:
> > *** BLURB HERE ***
> 
> Really?
> 
> And your subject line could use some work too :(
>

sorry for that, i've made a script for sending patchset and accidently 
it sents mails without contorl.
Fixed that
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/4] staging: android: Replace BUG_ON with WARN_ON

[Tomer Samara]

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 
> > ---
> >  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 :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 0/2] staging: android: Remove BUG/BUG_ON from ion

[Tomer Samara]

Removeing BUG/BUG_ON from androind/ion and add error handle to calling
functions

Tomer Samara (2):
  staging: android: Remove BUG_ON from ion_page_pool.c
  staging: android: Remove BUG from ion_system_heap.c

 drivers/staging/android/ion/ion_page_pool.c   | 14 ++
 drivers/staging/android/ion/ion_system_heap.c | 15 ---
 2 files changed, 22 insertions(+), 7 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 0/2] staging: android: Remove BUG/BUG_ONs

[Tomer Samara]

Remove BUG/BUG_ONs from androind/ion allocator and add error handling to
calling functions

Tomer Samara (2):
  staging: android: Remove BUG_ON from ion_page_pool.c
  staging: android: Remove BUG from ion_system_heap.c

 drivers/staging/android/ion/ion_page_pool.c   | 14 ++
 drivers/staging/android/ion/ion_system_heap.c | 15 ---
 2 files changed, 22 insertions(+), 7 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 1/2] staging: android: Remove BUG_ON from ion_page_pool.c

[Tomer Samara]

BUG_ON() is removed at ion_page_pool.c and add error handleing to
ion_page_pool_shrink

Fixes the following issue:
Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan BUG() 
or BUG_ON().

Signed-off-by: Tomer Samara 
---
 drivers/staging/android/ion/ion_page_pool.c | 14 ++
 1 file changed, 10 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..ae2bc57bcbe8 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 (!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 (!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 (!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 (pool->order != compound_order(page))
+   return;
 
ion_page_pool_add(pool, page);
 }
@@ -124,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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/2] staging: android: Remove BUG from ion_system_heap.c

[Tomer Samara]

Remove BUG()  at ion_sytem_heap.c and error handling to:
 - free_buffer_page
 - alloc_buffer_page
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 
---
 drivers/staging/android/ion/ion_system_heap.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index eac0632ab4e8..56d53268b82c 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
for (i = 0; i < NUM_ORDERS; i++)
if (order == orders[i])
return i;
-   BUG();
+
return -1;
 }
 
@@ -48,8 +48,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);
 }
 
@@ -58,6 +63,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) {
@@ -65,8 +71,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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 0/2] staging: android: Remove BUG/BUG_ON from ion

[Tomer Samara]

Remove BUG/BUG_ON from androind/ion


-v4:
some changes based on Dan Carpenter review:
- Remove error check at ion_page_pool_remove (conditions are impossible)
- Remove error check at opn_page_pool_alloc
- restore WARN_ON at ion_page_pool_free
- Remove unnecessary error check at ion_page_pool_shrink
- Add /* This is impossible. */ comment at order_to_index
- Remove error handling of order_to_index

-v3:
remove WARN/WARN_ON as Gerg KH suggests

-v2: 
add error check to order_to_index callers

Tomer Samara (2):
  staging: android: Remove BUG_ON from ion_page_pool.c
  staging: android: Remove BUG from ion_system_heap.c

 drivers/staging/android/ion/ion_page_pool.c   | 6 +-
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 1/2] staging: android: Remove BUG_ON from ion_page_pool.c

[Tomer Samara]

BUG_ON() is removed 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 
---
 drivers/staging/android/ion/ion_page_pool.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index 0198b886d906..a3ed3bfd47ee 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -46,11 +46,9 @@ static struct page *ion_page_pool_remove(struct 
ion_page_pool *pool, bool high)
struct page *page;
 
if (high) {
-   BUG_ON(!pool->high_count);
page = list_first_entry(&pool->high_items, struct page, lru);
pool->high_count--;
} else {
-   BUG_ON(!pool->low_count);
page = list_first_entry(&pool->low_items, struct page, lru);
pool->low_count--;
}
@@ -65,8 +63,6 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
 {
struct page *page = NULL;
 
-   BUG_ON(!pool);
-
mutex_lock(&pool->mutex);
if (pool->high_count)
page = ion_page_pool_remove(pool, true);
@@ -82,7 +78,7 @@ 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));
+   WARN_ON(pool->order != compound_order(page))
 
ion_page_pool_add(pool, page);
 }
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 2/2] staging: android: Remove BUG from ion_system_heap.c

[Tomer Samara]

Remove BUG() from 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 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index eac0632ab4e8..00d6154aec34 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
for (i = 0; i < NUM_ORDERS; i++)
if (order == orders[i])
return i;
-   BUG();
+   /* This is impossible. */
return -1;
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 2/2] staging: android: Remove BUG from ion_system_heap.c

[Tomer Samara]

On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> On 8/21/20 8:28 AM, Tomer Samara wrote:
> > Remove BUG() from 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 
> > ---
> >  drivers/staging/android/ion/ion_system_heap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> > b/drivers/staging/android/ion/ion_system_heap.c
> > index eac0632ab4e8..00d6154aec34 100644
> > --- a/drivers/staging/android/ion/ion_system_heap.c
> > +++ b/drivers/staging/android/ion/ion_system_heap.c
> > @@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
> > for (i = 0; i < NUM_ORDERS; i++)
> > if (order == orders[i])
> > return i;
> > -   BUG();
> > +   /* This is impossible. */
> > return -1;
> >  }
> 
> Hi,
> Please explain why this is impossible.
> 
> If some caller calls order_to_index(5), it will return -1, yes?
> 
> -- 
> ~Randy
> 

As Dan Carpenter says here 
https://lkml.kernel.org/lkml/cover.1597865771.git.tomersamar...@gmail.com/T/#mc790b91029565b1bb0cb87997b39007d9edb6e04.
After looking at callers we see that order_to_index called from 2 functions:
- alloc_buffer_page called from alloc_largest_available which 
  loop over all legit order nubmers
  ( Flow:
   alloc_largest_available-->alloc_buffer_page-->order_to_index
  )

- free_buffer_page takes the order using compound_order, which return 0 or
  the order number for the page, this function has 2 callers too,
  ion_system_heap_allocate (which called it in case of failure at 
sg_alloc_table,
  thus calling from this flow will no casue error) and ion_system_heap_free
  (which will be called on every sg table in the buffer that allocated good,
  meaning from this flow also error will not be created).
  ( Flows:
   ion_system_heap_free --> free_buffer_page --> order_to_index
   ion_system_heap_allocate --> free_buffer_page --> order_to_index
  )

Of course if some user will use this function with wrong order number he will 
be able to get this -1.
So should I remove this comment and resotre the error checks?
Btw, this is the same reason that I dropped the error check at 
ion_page_pool_shrink, so should I restore here also?

Thanks,
Tomer Samara

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 0/2] staging: android: Remove BUG/BUG_ON

[Tomer Samara]

Remove BUG/BUG_ON from androind/ion

-v5:
remove WARN_ON from ion_page_pool_free

-v4:
some changes based on Dan Carpenter review:
- Remove error check at ion_page_pool_remove (conditions are impossible)
- Remove error check at opn_page_pool_alloc
- restore WARN_ON at ion_page_pool_free
- Remove unnecessary error check at ion_page_pool_shrink
- Add /* This is impossible. */ comment at order_to_index
- Remove error handling of order_to_index

-v3:
remove WARN/WARN_ON as Gerg KH suggests

-v2: 
add error check to order_to_index callers

Tomer Samara (2):
  staging: android: Remove BUG_ON from ion_page_pool.c
  staging: android: Remove BUG from ion_system_heap.c

 drivers/staging/android/ion/ion_page_pool.c   | 6 --
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 1/2] staging: android: Remove BUG_ON from ion_page_pool.c

[Tomer Samara]

BUG_ON() is removed 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 
---
 drivers/staging/android/ion/ion_page_pool.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index 0198b886d906..fa764299f004 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -46,11 +46,9 @@ static struct page *ion_page_pool_remove(struct 
ion_page_pool *pool, bool high)
struct page *page;
 
if (high) {
-   BUG_ON(!pool->high_count);
page = list_first_entry(&pool->high_items, struct page, lru);
pool->high_count--;
} else {
-   BUG_ON(!pool->low_count);
page = list_first_entry(&pool->low_items, struct page, lru);
pool->low_count--;
}
@@ -65,8 +63,6 @@ struct page *ion_page_pool_alloc(struct ion_page_pool *pool)
 {
struct page *page = NULL;
 
-   BUG_ON(!pool);
-
mutex_lock(&pool->mutex);
if (pool->high_count)
page = ion_page_pool_remove(pool, true);
@@ -82,8 +78,6 @@ 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));
-
ion_page_pool_add(pool, page);
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v5 2/2] staging: android: Remove BUG from ion_system_heap.c

[Tomer Samara]

Remove BUG() from 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 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index eac0632ab4e8..00d6154aec34 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
for (i = 0; i < NUM_ORDERS; i++)
if (order == orders[i])
return i;
-   BUG();
+   /* This is impossible. */
return -1;
 }
 
-- 
2.25.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel