[PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[Gerd Bayer]

This reverts commit 58effa3476536215530c9ec4910ffc981613b413.
Review was not finished on this patch. So it's not ready for
upstreaming.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---

Hi,

this is broken for a little while already and I'd rather have the full 
solution properly reviewed and acked in one patch. So please do not upstream
this yet.

Thank you,
Gerd

 drivers/s390/net/ism_drv.c | 38 +++++++++-----------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index affb05521e14..2c8e964425dc 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -14,8 +14,6 @@
 #include <linux/err.h>
 #include <linux/ctype.h>
 #include <linux/processor.h>
-#include <linux/dma-mapping.h>
-#include <linux/mm.h>
 
 #include "ism.h"
 
@@ -294,15 +292,13 @@ static int ism_read_local_gid(struct ism_dev *ism)
 static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	clear_bit(dmb->sba_idx, ism->sba_bitmap);
-	dma_unmap_page(&ism->pdev->dev, dmb->dma_addr, dmb->dmb_len,
-		       DMA_FROM_DEVICE);
-	folio_put(virt_to_folio(dmb->cpu_addr));
+	dma_free_coherent(&ism->pdev->dev, dmb->dmb_len,
+			  dmb->cpu_addr, dmb->dma_addr);
 }
 
 static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	unsigned long bit;
-	int rc;
 
 	if (PAGE_ALIGN(dmb->dmb_len) > dma_get_max_seg_size(&ism->pdev->dev))
 		return -EINVAL;
@@ -319,30 +315,14 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 	    test_and_set_bit(dmb->sba_idx, ism->sba_bitmap))
 		return -EINVAL;
 
-	dmb->cpu_addr =
-		folio_address(folio_alloc(GFP_KERNEL | __GFP_NOWARN |
-					  __GFP_NOMEMALLOC | __GFP_NORETRY,
-					  get_order(dmb->dmb_len)));
+	dmb->cpu_addr = dma_alloc_coherent(&ism->pdev->dev, dmb->dmb_len,
+					   &dmb->dma_addr,
+					   GFP_KERNEL | __GFP_NOWARN |
+					   __GFP_NOMEMALLOC | __GFP_NORETRY);
+	if (!dmb->cpu_addr)
+		clear_bit(dmb->sba_idx, ism->sba_bitmap);
 
-	if (!dmb->cpu_addr) {
-		rc = -ENOMEM;
-		goto out_bit;
-	}
-	dmb->dma_addr = dma_map_page(&ism->pdev->dev,
-				     virt_to_page(dmb->cpu_addr), 0,
-				     dmb->dmb_len, DMA_FROM_DEVICE);
-	if (dma_mapping_error(&ism->pdev->dev, dmb->dma_addr)) {
-		rc = -ENOMEM;
-		goto out_free;
-	}
-
-	return 0;
-
-out_free:
-	kfree(dmb->cpu_addr);
-out_bit:
-	clear_bit(dmb->sba_idx, ism->sba_bitmap);
-	return rc;
+	return dmb->cpu_addr ? 0 : -ENOMEM;
 }
 
 int ism_register_dmb(struct ism_dev *ism, struct ism_dmb *dmb,

base-commit: b46f4eaa4f0ec38909fb0072eea3aeddb32f954e
-- 
2.44.0

Re: [PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[Paolo Abeni]

Hi,

On Tue, 2024-04-09 at 13:37 +0200, Gerd Bayer wrote:
> This reverts commit 58effa3476536215530c9ec4910ffc981613b413.
> Review was not finished on this patch. So it's not ready for
> upstreaming.
> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>

It's not a big deal (no need to repost), but should the need arise
again in the future it would be better explicitly marking the reverted
commit in the tag area as 'Fixes'. The full hash in the commit message
will likely save the day to stable teams, but better safe then sorry!

Thanks,

Paolo

Re: [PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue,  9 Apr 2024 13:37:53 +0200 you wrote:
> This reverts commit 58effa3476536215530c9ec4910ffc981613b413.
> Review was not finished on this patch. So it's not ready for
> upstreaming.
> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> ---
> 
> [...]

Here is the summary with links:
  - [net] Revert "s390/ism: fix receive message buffer allocation"
    https://git.kernel.org/netdev/net/c/d51dc8dd6ab6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[Gerd Bayer]

On Thu, 2024-04-11 at 09:16 +0200, Paolo Abeni wrote:
> Hi,
> 
> On Tue, 2024-04-09 at 13:37 +0200, Gerd Bayer wrote:
> > This reverts commit 58effa3476536215530c9ec4910ffc981613b413.
> > Review was not finished on this patch. So it's not ready for
> > upstreaming.
> > 
> > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> 
> It's not a big deal (no need to repost), but should the need arise
> again in the future it would be better explicitly marking the
> reverted commit in the tag area as 'Fixes'. The full hash in the
> commit message will likely save the day to stable teams, but better
> safe then sorry!

Thanks Paolo for the explanation. I was not even sure if the commit
hash of the erroneous commit will remain stable when this tree will be
merged upstream. In my (naive?) view this could be "autosquashed" into
nothing at the time of the merge.

But since there appears to be time for the next pull request to
upstream, I'll send a new version of the original patch with all the
review comments addressed.

Thanks again,
Gerd

Re: [PATCH net] Revert "s390/ism: fix receive message buffer allocation"

[Simon Horman]

On Thu, Apr 11, 2024 at 11:13:53AM +0200, Gerd Bayer wrote:
> On Thu, 2024-04-11 at 09:16 +0200, Paolo Abeni wrote:
> > Hi,
> > 
> > On Tue, 2024-04-09 at 13:37 +0200, Gerd Bayer wrote:
> > > This reverts commit 58effa3476536215530c9ec4910ffc981613b413.
> > > Review was not finished on this patch. So it's not ready for
> > > upstreaming.
> > > 
> > > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > 
> > It's not a big deal (no need to repost), but should the need arise
> > again in the future it would be better explicitly marking the
> > reverted commit in the tag area as 'Fixes'. The full hash in the
> > commit message will likely save the day to stable teams, but better
> > safe then sorry!
> 
> Thanks Paolo for the explanation. I was not even sure if the commit
> hash of the erroneous commit will remain stable when this tree will be
> merged upstream. In my (naive?) view this could be "autosquashed" into
> nothing at the time of the merge.

net and net-next hashes are always stable.

> But since there appears to be time for the next pull request to
> upstream, I'll send a new version of the original patch with all the
> review comments addressed.

It looks like this commit was accepted, so we have what it is.