All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Daniel Walker <danielwa@cisco.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	George Cherian <george.cherian@marvell.com>,
	sgoutham@marvell.com, "BOBBY Liu (bobbliu)" <bobbliu@cisco.com>,
	xe-linux-external@cisco.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] genirq: allow selection of number of sparse irqs
Date: Wed, 03 Aug 2022 08:16:20 +0100	[thread overview]
Message-ID: <87sfmdbxvf.wl-maz@kernel.org> (raw)
In-Reply-To: <20220802223747.GX821407@zorba>

On Tue, 02 Aug 2022 23:37:47 +0100,
Daniel Walker <danielwa@cisco.com> wrote:
> 
> On Sat, Jul 30, 2022 at 10:59:05AM +0100, Marc Zyngier wrote:
> > > 
> > > Marvell submitted a similar change, but non-selectable, about a
> > > month ago.
> > 
> > Which wasn't really acceptable either.
> > 
> > > 
> > > The limitation prevents Cisco and Marvell hardware from
> > > functioning. I don't think we're well versed enough on the generic
> > > irq system to implement what your suggesting, even if we did Thomas
> > > would not likely accept it.
> > 
> > I don't think you can speak for Thomas here. In my experience of
> > working with him, he's in general much more inclined to look at a
> > scalable, long term solution than at a point hack. Specially given
> > that we already use xarrays for MSIs.
>  
> Your welcome make the attempt yourself, if you believe in it.

The thing is, I don't need it, while you apparently do need a change
in the kernel.

> 
> > > Your suggestion is more of a long term solution vs. our short term
> > > solution.
> > 
> > Exactly. Experience shows that short term hacks are almost always a
> > bad idea and result in something that isn't maintainable.
> 
> Thomas introduced the "hack" in c1ee626 in 2011.

Yes. And it covers all the systems we care about so far. It is small,
fixed in size, and doesn't impose extra requirements on everyone else.
Your system changes the requirement, and it is the opportunity to
revisit an 11 year old decision.

> It's more of a question of if someone has the time an and/or
> inclination to make the changes your requesting.

No, it is about who has the need. You do, and nobody else does.

> Marvell and Cisco only require to increase the size and keep the
> status quo, and nothing is wrong with that.

It is pretty wrong when it adds unneeded overhead on systems that
don't require this, and doesn't scale in the face of existing
architectures (let alone future ones). Distributions ship a single
kernel image, and would obviously select the largest possible value,
just to maximise perceived compatibility requirements. My ask is that
you don't inflict this on systems that do not need it.

> 
> > > I'm not wedded to any solution, we just need to relieve
> > > the limitation so our hardware starts working. I would imagine other
> > > companies have this issue, but I don't know which ones currently.
> > 
> > This architecture has been in the wild for the best part of 10 years,
> > in Linux for 8 years, and nobody so far screamed because of this
> > perceived limitation. It would help if you described exactly what
> > breaks in your system, because just saying "give me more" is not
> > exactly helping (there are other limitations in the GICv3 ITS driver
> > that may bite you anyway).
> 
> We need more irq lines because we have a lot of devices.. I suppose it's
> possible there's some defect in the kernel which eats up or wastes irq lines,
> but I don't think so. We have devices which use a lot of irq lines.
> 
> > > I would rather to use an upstream solution verses holding the
> > > patches privately.  I would suggest if this limitation would not be
> > > overcome for 3-4 releases the short term solution should be
> > > acceptable over that time frame to be replaced by something else
> > > after that.
> > 
> > If you want to have an impact on the features being merged in the
> > upstream kernel, a good start would be to take feedback on board.
> 
> We did that.. I updated the patch from Marvell's original to allow it to be
> selectable, this was requested by someone on this list.

Well, I'm another "someone on the list" asking you to do better. You
are perfectly entitled to ignore me, and I'm just as entitled to voice
my opposition to your approach.

	M.

-- 
Without deviation from the norm, progress is not possible.

  reply	other threads:[~2022-08-03  7:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-28  3:04 [PATCH] genirq: allow selection of number of sparse irqs Daniel Walker
2022-07-28  8:52 ` Marc Zyngier
2022-07-29 18:21   ` Daniel Walker
2022-07-30  9:59     ` Marc Zyngier
2022-08-02 22:37       ` Daniel Walker
2022-08-03  7:16         ` Marc Zyngier [this message]
2022-08-03 22:44           ` Daniel Walker

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=87sfmdbxvf.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=bobbliu@cisco.com \
    --cc=danielwa@cisco.com \
    --cc=george.cherian@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sgoutham@marvell.com \
    --cc=tglx@linutronix.de \
    --cc=xe-linux-external@cisco.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.