On Fri, 2024-04-05 at 08:49 +0200, Christoph Hellwig wrote:
> On Thu, Apr 04, 2024 at 01:10:20PM +0200, Gerd Bayer wrote:
> > > Why can't you use get_free_pages() (or similar) here? (possibly
> > > rounding up to the relevant page_aligned size).
> >
> > Thanks Paolo for your suggestion. However, I wanted to stay as
> > close to the implementation pre [1] - that used to use __GFP_COMP,
> > too. I'd rather avoid to change interfaces from "cpu_addr" to
> > "struct page*" at this point. In the long run, I'd like to drop the
> > requirement for
>
> The right interface actually is to simply use folio_alloc, which adds
> __GFP_COMP and is a fully supported and understood interface. You can
> just convert the folio to a kernel virtual address using
> folio_address() right after allocating it.
Thanks for pointing me to folios.
After a good night's sleep, I figured that I was thinking too
complicated when I dismissed Paolo's suggestion.
> (get_free_pages also retunrs a kernel virtual address, just awkwardly
> as an unsigned long. In doubt don't use this interface for new
> code..)
>
> > compound pages entirely, since that *appears* to exist primarily
> > for a
> > simplified handling of the interface to splice_to_pipe() in
> > net/smc/smc_rx.c. And of course there might be performance
> > implications...
>
> While compounds pages might sound awkward, they are the new normal in
> form of folios. So just use folios.
With the following fixup, my tests were just as successful.
I'll send that out as a v2.
Thank you, Christoph and Paolo!
diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index 25911b887e5e..affb05521e14 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -14,8 +14,8 @@
#include <linux/err.h>
#include <linux/ctype.h>
#include <linux/processor.h>
-#include <linux/dma-direction.h>
-#include <linux/gfp_types.h>
+#include <linux/dma-mapping.h>
+#include <linux/mm.h>
#include "ism.h"
@@ -296,7 +296,7 @@ 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);
- kfree(dmb->cpu_addr);
+ folio_put(virt_to_folio(dmb->cpu_addr));
}
static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
@@ -319,8 +319,11 @@ 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 = kmalloc(dmb->dmb_len, GFP_KERNEL |
__GFP_NOWARN |
- __GFP_COMP | __GFP_NOMEMALLOC |
__GFP_NORETRY);
+ dmb->cpu_addr =
+ folio_address(folio_alloc(GFP_KERNEL | __GFP_NOWARN |
+ __GFP_NOMEMALLOC |
__GFP_NORETRY,
+ get_order(dmb->dmb_len)));
+
if (!dmb->cpu_addr) {
rc = -ENOMEM;
goto out_bit;
--
2.44.0
