public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: robert.richter@cavium.com (Robert Richter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] irqchip: gicv3-its: Use NUMA aware memory allocation for ITS tables
Date: Thu, 13 Jul 2017 17:40:00 +0200	[thread overview]
Message-ID: <20170713154000.GA18024@rric.localdomain> (raw)
In-Reply-To: <20170711084855.GA4193@localhost>

On 11.07.17 08:48:56, Jayachandran C wrote:
> On Mon, Jul 10, 2017 at 04:15:28PM +0100, Marc Zyngier wrote:
> > On 10/07/17 15:57, Shanker Donthineni wrote:

> > > I believe ITS driver should provide NUMA aware allocations just like x86 Linux drivers. How much
> > > performance improvement we observer is based on the individual SOC implementation, inter NODE
> > > latency, inter node traffic, cache capacity, and type of the test used to measure results.
> > > 
> > > Please consider this patch irrespective of the test results running on a specific hardware. We
> > > need this patch for upcoming Qualcomm server chips. 
> > 
> > "I believe" and "We need" are not a proof of the usefulness of this. We
> > can argue all day, or you can provide a set of convincing results. Your
> > choice. But I can guarantee you the the latter is a much better method
> > than the former.
> > 
> > If you (or Cavium) cannot be bothered to provide tangible results that
> > this is useful, why should I take this at face value? This is just like
> > any other improvement we make to the kernel. We back it *with data*.
> 
> At Cavium, most of the ThunderX2 boards we have are multi-node, and we
> are interested in enabling NUMA optimizations.
> 
> But, in this case, we do not see (or expect to see - given the nature of
> access) any significant improvement in any standard benchmark. Ganapat's
> LPI injection test to find interrupt latency was probably the best option
> we had so far. We could come up with another contrived test case to see
> if there is any change in behavior when we overload the interconnect,
> but I don't think we will get any data to really justify the patch.
> 
> Allocating the tables on the node is a good thing since it avoids
> unnecessary traffic over the interconnect, so I do not see the
> problem in merging a simple patch for that. Is there any specific
> issue here?
> 
> Anyway, for ThunderX2, the patch is good to have, but not critical.
> And as Ganapat noted, the patch can be improved a bit. Also going thru
> the patch, I think the chip data is better allocated using node as well.

There is another thing to consider here. We will need cma and devm for
ITS. There are only a few per node allocation functions that can be
used then, so per-node allocation should only be used in rare cases
where really needed. I am going to repost my cma device table
allocation series after the merge window closes.

-Robert

  reply	other threads:[~2017-07-13 15:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-25 15:46 [PATCH] irqchip: gicv3-its: Use NUMA aware memory allocation for ITS tables Shanker Donthineni
2017-06-30  2:34 ` Ganapatrao Kulkarni
2017-06-30  3:01   ` Ganapatrao Kulkarni
2017-06-30  8:51     ` Marc Zyngier
2017-07-03 14:24       ` Shanker Donthineni
2017-07-03 14:53         ` Marc Zyngier
2017-07-03 15:15           ` Shanker Donthineni
2017-07-10  8:48           ` Ganapatrao Kulkarni
2017-07-10  9:06             ` Marc Zyngier
2017-07-10  9:08               ` Ganapatrao Kulkarni
2017-07-10  9:23                 ` Marc Zyngier
2017-07-10 10:21                   ` Ganapatrao Kulkarni
2017-07-10 12:30                     ` Shanker Donthineni
2017-07-10 13:53                       ` Marc Zyngier
2017-07-10 13:50                     ` Marc Zyngier
2017-07-10 14:57                       ` Shanker Donthineni
2017-07-10 15:15                         ` Marc Zyngier
2017-07-11  8:48                           ` Jayachandran C
2017-07-13 15:40                             ` Robert Richter [this message]
2017-07-03 13:37     ` Shanker Donthineni
2017-07-03 16:51       ` Ganapatrao Kulkarni
2017-07-03 18:25         ` Shanker Donthineni
2017-07-03 13:30   ` Shanker Donthineni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170713154000.GA18024@rric.localdomain \
    --to=robert.richter@cavium.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox