[PATCH 1/1] Fixed to BUG_ON to WARN_ON def

Hello all, 
I think should be use to "WARN_ON" and checkpatch script give to error, I fixed and I think  should don't use "BUG_ON".
Regards,

Signed-off-by: Ozgur Karatas <redacted>
---
drivers/net/ethernet/mellanox/mlx4/icm.c |  4 ++--

diff --git a/drivers/net/ethernet/mellanox/mlx4/icm.c b/drivers/net/ethernet/mellanox/mlx4/icm.c
index 2a9dd46..3fde535 100644
--- a/drivers/net/ethernet/mellanox/mlx4/icm.c
+++ b/drivers/net/ethernet/mellanox/mlx4/icm.c

@@ -119,7 +119,7 @@ static int mlx4_alloc_icm_coherent(struct device *dev, struct scatterlist *mem,
 		return -ENOMEM;
 
 	sg_set_buf(mem, buf, PAGE_SIZE << order);
-	BUG_ON(mem->offset);
+	WARN_ON(mem->offset);
 	sg_dma_len(mem) = PAGE_SIZE << order;
 	return 0;
 }

@@ -133,7 +133,7 @@ struct mlx4_icm *mlx4_alloc_icm(struct mlx4_dev *dev, int npages,
 	int ret;
 
 	/* We use sg_set_buf for coherent allocs, which assumes low memory */
-	BUG_ON(coherent && (gfp_mask & __GFP_HIGHMEM));
+	WARN_ON(coherent && (gfp_mask & __GFP_HIGHMEM));
 
 	icm = kmalloc_node(sizeof(*icm),
 			   gfp_mask & ~(__GFP_HIGHMEM | __GFP_NOWARN),

-- 
2.1.4

Hello.

On 12/12/16 11:58, Ozgur Karatas wrote:

Hello all,
I think should be use to "WARN_ON" and checkpatch script give to error, I fixed and I think  should don't use "BUG_ON".
Regards,

Signed-off-by: Ozgur Karatas <redacted>

I pointed you already before to the Documentation how to prepare a 
commit subject and commit message. You just replied with that you are 
new to contributing patches. That is all fine and many people are new 
each release. Please take the time to read the provided and pointed out 
docs.

If you keep ignoring such suggestions and docs I would think people will 
keep ignoring your patches.

regards
Stefan Schmidt

Dear Stefan;

I'm reading to Documentation/SubmittingPatches and I still apologized for misrepresentations my patches. 
I will add a next time good commit message and commit subjects.

Sorry,

Regards

Ozgur Karatas

12.12.2016, 13:20, "Stefan Schmidt" [off-list ref]:

Hello.

On 12/12/16 11:58, Ozgur Karatas wrote:

quoted

 Hello all,
 I think should be use to "WARN_ON" and checkpatch script give to error, I fixed and I think should don't use "BUG_ON".
 Regards,

 Signed-off-by: Ozgur Karatas [off-list ref]

I pointed you already before to the Documentation how to prepare a
commit subject and commit message. You just replied with that you are
new to contributing patches. That is all fine and many people are new
each release. Please take the time to read the provided and pointed out
docs.

If you keep ignoring such suggestions and docs I would think people will
keep ignoring your patches.

regards
Stefan Schmidt

On Mon, Dec 12, 2016 at 12:58:59PM +0200, Ozgur Karatas wrote:

Hello all,
I think should be use to "WARN_ON" and checkpatch script give to error, I fixed and I think  should don't use "BUG_ON".
Regards,

Signed-off-by: Ozgur Karatas <redacted>

NAK, Leon Romanovsky [off-list ref]

If we put aside commit message issue, which was pointed to you by Stefan, your
proposed change is incorrect. By chnaging BUG_ONs to be WARN_ONs, you
will left the driver in improper state.

Thanks

Dear Romanovsky;

I'm trying to learn english and I apologize for my mistake words and phrases. So, I think the code when call to "sg_set_buf" and next time set memory and buffer. For example, isn't to call "WARN_ON" function, get a error to implicit declaration, right?

Because, you will use to "BUG_ON" get a error implicit declaration of functions.

        sg_set_buf(mem, buf, PAGE_SIZE << order);
        WARN_ON(mem->offset);

Thanks for information and learn to me.

Regards,

Ozgur Karatas

12.12.2016, 14:39, "Leon Romanovsky" [off-list ref]:

On Mon, Dec 12, 2016 at 12:58:59PM +0200, Ozgur Karatas wrote:

quoted

 Hello all,
 I think should be use to "WARN_ON" and checkpatch script give to error, I fixed and I think should don't use "BUG_ON".
 Regards,

 Signed-off-by: Ozgur Karatas [off-list ref]

NAK, Leon Romanovsky [off-list ref]

If we put aside commit message issue, which was pointed to you by Stefan, your
proposed change is incorrect. By chnaging BUG_ONs to be WARN_ONs, you
will left the driver in improper state.

Thanks

quoted

 ---
 drivers/net/ethernet/mellanox/mlx4/icm.c | 4 ++--

 diff --git a/drivers/net/ethernet/mellanox/mlx4/icm.c b/drivers/net/ethernet/mellanox/mlx4/icm.c
 index 2a9dd46..3fde535 100644
 --- a/drivers/net/ethernet/mellanox/mlx4/icm.c
 +++ b/drivers/net/ethernet/mellanox/mlx4/icm.c
 @@ -119,7 +119,7 @@ static int mlx4_alloc_icm_coherent(struct device *dev, struct scatterlist *mem,
                  return -ENOMEM;

          sg_set_buf(mem, buf, PAGE_SIZE << order);
 - BUG_ON(mem->offset);
 + WARN_ON(mem->offset);
          sg_dma_len(mem) = PAGE_SIZE << order;
          return 0;
  }
 @@ -133,7 +133,7 @@ struct mlx4_icm *mlx4_alloc_icm(struct mlx4_dev *dev, int npages,
          int ret;

          /* We use sg_set_buf for coherent allocs, which assumes low memory */
 - BUG_ON(coherent && (gfp_mask & __GFP_HIGHMEM));
 + WARN_ON(coherent && (gfp_mask & __GFP_HIGHMEM));

          icm = kmalloc_node(sizeof(*icm),
                             gfp_mask & ~(__GFP_HIGHMEM | __GFP_NOWARN),
 --
 2.1.4

On Mon, Dec 12, 2016 at 03:04:28PM +0200, Ozgur Karatas wrote:

Dear Romanovsky;

Please avoid top-posting in your replies.
Thanks

I'm trying to learn english and I apologize for my mistake words and phrases. So, I think the code when call to "sg_set_buf" and next time set memory and buffer. For example, isn't to call "WARN_ON" function, get a error to implicit declaration, right?

Because, you will use to "BUG_ON" get a error implicit declaration of functions.

I'm not sure that I followed you. mem->offset is set by sg_set_buf from
buf variable returned by dma_alloc_coherent(). HW needs to get very
precise size of this buf, in multiple of pages and aligned to pages
boundaries.

        sg_set_buf(mem, buf, PAGE_SIZE << order);
        WARN_ON(mem->offset);

See the patch inline which removes this BUG_ON in proper and safe way.

From 7babe807affa2b27d51d3610afb75b693929ea1a Mon Sep 17 00:00:00 2001
From: Leon Romanovsky <redacted>
Date: Mon, 12 Dec 2016 20:02:45 +0200
Subject: [PATCH] net/mlx4: Remove BUG_ON from ICM allocation routine

This patch removes BUG_ON() macro from mlx4_alloc_icm_coherent()
by checking DMA address aligment in advance and performing proper
folding in case of error.

Fixes: 5b0bf5e25efe ("mlx4_core: Support ICM tables in coherent memory")
Reported-by: Ozgur Karatas <redacted>
Signed-off-by: Leon Romanovsky <redacted>
---
 drivers/net/ethernet/mellanox/mlx4/icm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/icm.c b/drivers/net/ethernet/mellanox/mlx4/icm.c
index 2a9dd46..e1f9e7c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/icm.c
+++ b/drivers/net/ethernet/mellanox/mlx4/icm.c

@@ -118,8 +118,13 @@ static int mlx4_alloc_icm_coherent(struct device *dev, struct scatterlist *mem,
 	if (!buf)
 		return -ENOMEM;

+	if (offset_in_page(buf)) {
+		dma_free_coherent(dev, PAGE_SIZE << order,
+				  buf, sg_dma_address(mem));
+		return -ENOMEM;
+	}
+
 	sg_set_buf(mem, buf, PAGE_SIZE << order);
-	BUG_ON(mem->offset);
 	sg_dma_len(mem) = PAGE_SIZE << order;
 	return 0;
 }
--

2.10.2

12.12.2016, 20:18, "Leon Romanovsky" [off-list ref]:

On Mon, Dec 12, 2016 at 03:04:28PM +0200, Ozgur Karatas wrote:

quoted

 Dear Romanovsky;

Please avoid top-posting in your replies.
Thanks

Dear Leon; 
thanks for the information., I will pay attention.

quoted

 I'm trying to learn english and I apologize for my mistake words and phrases. So, I think the code when call to "sg_set_buf" and next time set memory and buffer. For example, isn't to call "WARN_ON" function, get a error to implicit declaration, right?

 Because, you will use to "BUG_ON" get a error implicit declaration of functions.

I'm not sure that I followed you. mem->offset is set by sg_set_buf from
buf variable returned by dma_alloc_coherent(). HW needs to get very
precise size of this buf, in multiple of pages and aligned to pages
boundaries.

I have studied the following your coding and I guess that's the right patchs.
You are the very expert in this matter, thank you for the correct for me.

I learn to your style as an example.

Regards,

Ozgur Karatas

Thanks Ozgur for your report.


On 12/12/2016 8:18 PM, Leon Romanovsky wrote:

Thanks Leon for the patch. It is the right way to do so.
Reviewed-by: Tariq Toukan <redacted>

We will submit Leon's patch in a new email.

Regards,
Tariq

2.10.2