From: Marc Zyngier <hidden> Date: 2018-08-26 19:01:30
[I'm travelling, so expect some major delays in responding to email]
Hi Jia,
On Sun, 26 Aug 2018 10:00:51 +0100,
Jia He [off-list ref] wrote:
There is a WARN_ON when my QDF2400 server boots up (pagesize is 4k)
[snip]
In its_alloc_lpi_tables, lpi_id_bits is 24, without this patch,
its_allocate_prop_table will try to allocate 16M(order 12 if
pagesize=4k). Thus it causes the WARN_ON.
Gah! QDF and its 24bit INTIDs... Making life hell for everyone ;-)
Sorry for breaking it.
quoted hunk
This patch fixes it by limiting the lpi_id_bits.
Signed-off-by: Jia He <redacted>
---
drivers/irqchip/irq-gic-v3-its.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
@@ -1624,8 +1624,11 @@ static void its_free_prop_table(struct page *prop_page)staticint__initits_alloc_lpi_tables(void){phys_addr_tpaddr;+u32max_bits;/*max order limitation in alloc_page*/-lpi_id_bits=GICD_TYPER_ID_BITS(gic_rdists->gicd_typer);+max_bits=PAGE_SHIFT+MAX_ORDER-1;+lpi_id_bits=min_t(u32,max_bits,+GICD_TYPER_ID_BITS(gic_rdists->gicd_typer));gic_rdists->prop_page=its_allocate_prop_table(GFP_NOWAIT);if(!gic_rdists->prop_page){pr_err("Failed to allocate PROPBASE\n");
--
1.8.3.1
I find it rather odd that we end-up with different interrupt ranges
depending on the CPU page size. Also, allocating that much memory for
LPIs is rather pointless, as we actually have a pretty low limit of
interrupts the system can deal with (see IRQ_BITMAP_BITS, which is
slightly more than 8k). I've so far seen *one* request to push it up,
but I doubt that it is a real use case.
Capping lpi_id_bits at 16 (which is what we had before) is plenty,
will save a some memory, and gives some margin before we need to push
it up again.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
Hi Marc
Thanks for the comments
On 8/27/2018 3:01 AM, Marc Zyngier Wrote:
[I'm travelling, so expect some major delays in responding to email]
Hi Jia,
On Sun, 26 Aug 2018 10:00:51 +0100,
Jia He [off-list ref] wrote:
quoted
There is a WARN_ON when my QDF2400 server boots up (pagesize is 4k)
[snip]
quoted
In its_alloc_lpi_tables, lpi_id_bits is 24, without this patch,
its_allocate_prop_table will try to allocate 16M(order 12 if
pagesize=4k). Thus it causes the WARN_ON.
Gah! QDF and its 24bit INTIDs... Making life hell for everyone ;-)
Sorry for breaking it.
np, maybe QDF2400 is a little bit special
quoted
This patch fixes it by limiting the lpi_id_bits.
Signed-off-by: Jia He <redacted>
---
drivers/irqchip/irq-gic-v3-its.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
@@ -1624,8 +1624,11 @@ static void its_free_prop_table(struct page *prop_page)staticint__initits_alloc_lpi_tables(void){phys_addr_tpaddr;+u32max_bits;/*max order limitation in alloc_page*/-lpi_id_bits=GICD_TYPER_ID_BITS(gic_rdists->gicd_typer);+max_bits=PAGE_SHIFT+MAX_ORDER-1;+lpi_id_bits=min_t(u32,max_bits,+GICD_TYPER_ID_BITS(gic_rdists->gicd_typer));gic_rdists->prop_page=its_allocate_prop_table(GFP_NOWAIT);if(!gic_rdists->prop_page){pr_err("Failed to allocate PROPBASE\n");
--
1.8.3.1
I find it rather odd that we end-up with different interrupt ranges
depending on the CPU page size. Also, allocating that much memory for
LPIs is rather pointless, as we actually have a pretty low limit of
interrupts the system can deal with (see IRQ_BITMAP_BITS, which is
slightly more than 8k). I've so far seen *one* request to push it up,
but I doubt that it is a real use case.
Capping lpi_id_bits at 16 (which is what we had before) is plenty,
will save a some memory, and gives some margin before we need to push
it up again.
Do you want me to revert commit fe8e93504 to cap the lpi_id_bits
to no greater than ITS_MAX_LPI_NRBITS(16) instead this patch?
--
Cheers,
Jia
From: Marc Zyngier <hidden> Date: 2018-08-27 09:10:01
On Mon, 27 Aug 2018 02:41:04 +0100,
Jia He [off-list ref] wrote:
Hi Marc
Thanks for the comments
On 8/27/2018 3:01 AM, Marc Zyngier Wrote:
quoted
[I'm travelling, so expect some major delays in responding to email]
Hi Jia,
On Sun, 26 Aug 2018 10:00:51 +0100,
Jia He [off-list ref] wrote:
quoted
There is a WARN_ON when my QDF2400 server boots up (pagesize is 4k)
[snip]
quoted
In its_alloc_lpi_tables, lpi_id_bits is 24, without this patch,
its_allocate_prop_table will try to allocate 16M(order 12 if
pagesize=4k). Thus it causes the WARN_ON.
Gah! QDF and its 24bit INTIDs... Making life hell for everyone ;-)
Sorry for breaking it.
np, maybe QDF2400 is a little bit special
quoted
quoted
This patch fixes it by limiting the lpi_id_bits.
Signed-off-by: Jia He <redacted>
---
drivers/irqchip/irq-gic-v3-its.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
@@ -1624,8 +1624,11 @@ static void its_free_prop_table(struct page *prop_page)staticint__initits_alloc_lpi_tables(void){phys_addr_tpaddr;+u32max_bits;/*max order limitation in alloc_page*/-lpi_id_bits=GICD_TYPER_ID_BITS(gic_rdists->gicd_typer);+max_bits=PAGE_SHIFT+MAX_ORDER-1;+lpi_id_bits=min_t(u32,max_bits,+GICD_TYPER_ID_BITS(gic_rdists->gicd_typer));gic_rdists->prop_page=its_allocate_prop_table(GFP_NOWAIT);if(!gic_rdists->prop_page){pr_err("Failed to allocate PROPBASE\n");
--
1.8.3.1
I find it rather odd that we end-up with different interrupt ranges
depending on the CPU page size. Also, allocating that much memory for
LPIs is rather pointless, as we actually have a pretty low limit of
interrupts the system can deal with (see IRQ_BITMAP_BITS, which is
slightly more than 8k). I've so far seen *one* request to push it up,
but I doubt that it is a real use case.
Capping lpi_id_bits at 16 (which is what we had before) is plenty,
will save a some memory, and gives some margin before we need to push
it up again.
Do you want me to revert commit fe8e93504 to cap the lpi_id_bits
to no greater than ITS_MAX_LPI_NRBITS(16) instead this patch?
Yes, this is probably the best course of action at this time. When I
dropped this, I was mostly thinking of the LPI allocator (which now
works with an almost constant memory footprint), and didn't pay enough
attention to the property table allocation (which is still a function
of the number of LPIs).
Thanks,
M.
--
Jazz is not dead, it just smell funny.