Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Tue, Jul 10, 2018 at 03:08:38PM +, Bart Van Assche wrote: > On Tue, 2018-07-10 at 08:39 +0300, Leon Romanovsky wrote: > > On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > > > An array of pointers to SRPT contexts in ib_device is over 30KiB even > > > in default case, in which an amount of contexts is 4095. The patch > > > is intended to weed out large contigous allocation for non-DMA memory. > > > > kvmalloc* doesn't "weed out" large contiguous allocations, but tries to > > allocate them and gracefully fallback to vmalloc if kmalloc fails. > > > > More on that, for allocations less than page (64KB for PowerPC), it will > > not call to vmalloc at all, which is fine too, because pages granularity. > > > > If you want to get rid of "contiguous allocations", use vmalloc instead. > > vmalloc() has a very significant disadvantage compared to kvmalloc(): with > vmalloc() it is guaranteed that memory allocations will be slow. That's not > the case for kvmalloc() if the number of bytes that is allocated is small > enough. Bart, I'm not arguing, just pointing that the commit message doesn't reflect actual change. Thanks > > Bart. > > > > signature.asc Description: PGP signature
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Tue, Jul 10, 2018 at 03:08:38PM +, Bart Van Assche wrote: > On Tue, 2018-07-10 at 08:39 +0300, Leon Romanovsky wrote: > > On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > > > An array of pointers to SRPT contexts in ib_device is over 30KiB even > > > in default case, in which an amount of contexts is 4095. The patch > > > is intended to weed out large contigous allocation for non-DMA memory. > > > > kvmalloc* doesn't "weed out" large contiguous allocations, but tries to > > allocate them and gracefully fallback to vmalloc if kmalloc fails. > > > > More on that, for allocations less than page (64KB for PowerPC), it will > > not call to vmalloc at all, which is fine too, because pages granularity. > > > > If you want to get rid of "contiguous allocations", use vmalloc instead. > > vmalloc() has a very significant disadvantage compared to kvmalloc(): with > vmalloc() it is guaranteed that memory allocations will be slow. That's not > the case for kvmalloc() if the number of bytes that is allocated is small > enough. Bart, I'm not arguing, just pointing that the commit message doesn't reflect actual change. Thanks > > Bart. > > > > signature.asc Description: PGP signature
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Tue, 2018-07-10 at 08:39 +0300, Leon Romanovsky wrote: > On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > > An array of pointers to SRPT contexts in ib_device is over 30KiB even > > in default case, in which an amount of contexts is 4095. The patch > > is intended to weed out large contigous allocation for non-DMA memory. > > kvmalloc* doesn't "weed out" large contiguous allocations, but tries to > allocate them and gracefully fallback to vmalloc if kmalloc fails. > > More on that, for allocations less than page (64KB for PowerPC), it will > not call to vmalloc at all, which is fine too, because pages granularity. > > If you want to get rid of "contiguous allocations", use vmalloc instead. vmalloc() has a very significant disadvantage compared to kvmalloc(): with vmalloc() it is guaranteed that memory allocations will be slow. That's not the case for kvmalloc() if the number of bytes that is allocated is small enough. Bart.
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Tue, 2018-07-10 at 08:39 +0300, Leon Romanovsky wrote: > On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > > An array of pointers to SRPT contexts in ib_device is over 30KiB even > > in default case, in which an amount of contexts is 4095. The patch > > is intended to weed out large contigous allocation for non-DMA memory. > > kvmalloc* doesn't "weed out" large contiguous allocations, but tries to > allocate them and gracefully fallback to vmalloc if kmalloc fails. > > More on that, for allocations less than page (64KB for PowerPC), it will > not call to vmalloc at all, which is fine too, because pages granularity. > > If you want to get rid of "contiguous allocations", use vmalloc instead. vmalloc() has a very significant disadvantage compared to kvmalloc(): with vmalloc() it is guaranteed that memory allocations will be slow. That's not the case for kvmalloc() if the number of bytes that is allocated is small enough. Bart.
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote:
> An array of pointers to SRPT contexts in ib_device is over 30KiB even
> in default case, in which an amount of contexts is 4095. The patch
> is intended to weed out large contigous allocation for non-DMA memory.
kvmalloc* doesn't "weed out" large contiguous allocations, but tries to
allocate them and gracefully fallback to vmalloc if kmalloc fails.
More on that, for allocations less than page (64KB for PowerPC), it will
not call to vmalloc at all, which is fine too, because pages granularity.
If you want to get rid of "contiguous allocations", use vmalloc instead.
Thanks
>
> Signed-off-by: Jan Dakinevich
> ---
> drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c
> b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 3081c62..de167af 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -720,7 +720,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
> srpt_device *sdev,
> WARN_ON(ioctx_size != sizeof(struct srpt_recv_ioctx)
> && ioctx_size != sizeof(struct srpt_send_ioctx));
>
> - ring = kmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
> + ring = kvmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
> if (!ring)
> goto out;
> for (i = 0; i < ring_size; ++i) {
> @@ -734,7 +734,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
> srpt_device *sdev,
> err:
> while (--i >= 0)
> srpt_free_ioctx(sdev, ring[i], dma_size, dir);
> - kfree(ring);
> + kvfree(ring);
> ring = NULL;
> out:
> return ring;
> @@ -759,7 +759,7 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx
> **ioctx_ring,
>
> for (i = 0; i < ring_size; ++i)
> srpt_free_ioctx(sdev, ioctx_ring[i], dma_size, dir);
> - kfree(ioctx_ring);
> + kvfree(ioctx_ring);
> }
>
> /**
> --
> 2.1.4
>
signature.asc
Description: PGP signature
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote:
> An array of pointers to SRPT contexts in ib_device is over 30KiB even
> in default case, in which an amount of contexts is 4095. The patch
> is intended to weed out large contigous allocation for non-DMA memory.
kvmalloc* doesn't "weed out" large contiguous allocations, but tries to
allocate them and gracefully fallback to vmalloc if kmalloc fails.
More on that, for allocations less than page (64KB for PowerPC), it will
not call to vmalloc at all, which is fine too, because pages granularity.
If you want to get rid of "contiguous allocations", use vmalloc instead.
Thanks
>
> Signed-off-by: Jan Dakinevich
> ---
> drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c
> b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 3081c62..de167af 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -720,7 +720,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
> srpt_device *sdev,
> WARN_ON(ioctx_size != sizeof(struct srpt_recv_ioctx)
> && ioctx_size != sizeof(struct srpt_send_ioctx));
>
> - ring = kmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
> + ring = kvmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
> if (!ring)
> goto out;
> for (i = 0; i < ring_size; ++i) {
> @@ -734,7 +734,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
> srpt_device *sdev,
> err:
> while (--i >= 0)
> srpt_free_ioctx(sdev, ring[i], dma_size, dir);
> - kfree(ring);
> + kvfree(ring);
> ring = NULL;
> out:
> return ring;
> @@ -759,7 +759,7 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx
> **ioctx_ring,
>
> for (i = 0; i < ring_size; ++i)
> srpt_free_ioctx(sdev, ioctx_ring[i], dma_size, dir);
> - kfree(ioctx_ring);
> + kvfree(ioctx_ring);
> }
>
> /**
> --
> 2.1.4
>
signature.asc
Description: PGP signature
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > An array of pointers to SRPT contexts in ib_device is over 30KiB even > in default case, in which an amount of contexts is 4095. The patch > is intended to weed out large contigous allocation for non-DMA memory. > > Signed-off-by: Jan Dakinevich > Reviewed-by: Bart Van Assche > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Applied to for-next Thanks, Jason
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, Jul 09, 2018 at 04:51:08PM +0300, Jan Dakinevich wrote: > An array of pointers to SRPT contexts in ib_device is over 30KiB even > in default case, in which an amount of contexts is 4095. The patch > is intended to weed out large contigous allocation for non-DMA memory. > > Signed-off-by: Jan Dakinevich > Reviewed-by: Bart Van Assche > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Applied to for-next Thanks, Jason
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, 2018-07-09 at 16:51 +0300, Jan Dakinevich wrote: > An array of pointers to SRPT contexts in ib_device is over 30KiB even > in default case, in which an amount of contexts is 4095. The patch > is intended to weed out large contigous allocation for non-DMA memory. Reviewed-by: Bart Van Assche
Re: [PATCH] ib_srpt: use kvmalloc to allocate ring pointers
On Mon, 2018-07-09 at 16:51 +0300, Jan Dakinevich wrote: > An array of pointers to SRPT contexts in ib_device is over 30KiB even > in default case, in which an amount of contexts is 4095. The patch > is intended to weed out large contigous allocation for non-DMA memory. Reviewed-by: Bart Van Assche
[PATCH] ib_srpt: use kvmalloc to allocate ring pointers
An array of pointers to SRPT contexts in ib_device is over 30KiB even
in default case, in which an amount of contexts is 4095. The patch
is intended to weed out large contigous allocation for non-DMA memory.
Signed-off-by: Jan Dakinevich
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c
b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 3081c62..de167af 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -720,7 +720,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
srpt_device *sdev,
WARN_ON(ioctx_size != sizeof(struct srpt_recv_ioctx)
&& ioctx_size != sizeof(struct srpt_send_ioctx));
- ring = kmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
+ ring = kvmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
if (!ring)
goto out;
for (i = 0; i < ring_size; ++i) {
@@ -734,7 +734,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
srpt_device *sdev,
err:
while (--i >= 0)
srpt_free_ioctx(sdev, ring[i], dma_size, dir);
- kfree(ring);
+ kvfree(ring);
ring = NULL;
out:
return ring;
@@ -759,7 +759,7 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx
**ioctx_ring,
for (i = 0; i < ring_size; ++i)
srpt_free_ioctx(sdev, ioctx_ring[i], dma_size, dir);
- kfree(ioctx_ring);
+ kvfree(ioctx_ring);
}
/**
--
2.1.4
[PATCH] ib_srpt: use kvmalloc to allocate ring pointers
An array of pointers to SRPT contexts in ib_device is over 30KiB even
in default case, in which an amount of contexts is 4095. The patch
is intended to weed out large contigous allocation for non-DMA memory.
Signed-off-by: Jan Dakinevich
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c
b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 3081c62..de167af 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -720,7 +720,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
srpt_device *sdev,
WARN_ON(ioctx_size != sizeof(struct srpt_recv_ioctx)
&& ioctx_size != sizeof(struct srpt_send_ioctx));
- ring = kmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
+ ring = kvmalloc_array(ring_size, sizeof(ring[0]), GFP_KERNEL);
if (!ring)
goto out;
for (i = 0; i < ring_size; ++i) {
@@ -734,7 +734,7 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct
srpt_device *sdev,
err:
while (--i >= 0)
srpt_free_ioctx(sdev, ring[i], dma_size, dir);
- kfree(ring);
+ kvfree(ring);
ring = NULL;
out:
return ring;
@@ -759,7 +759,7 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx
**ioctx_ring,
for (i = 0; i < ring_size; ++i)
srpt_free_ioctx(sdev, ioctx_ring[i], dma_size, dir);
- kfree(ioctx_ring);
+ kvfree(ioctx_ring);
}
/**
--
2.1.4
