2014-10-01 15:22:33

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

On Wed, Sep 24, 2014 at 10:39:13AM -0400, Tejun Heo wrote:
> Hello, Alexander.
>
> On Wed, Sep 24, 2014 at 03:08:44PM +0100, Alexander Gordeev wrote:
> > > Hmmm, how would the whole system benefit from it if there's only
> > > single device? Each individual servicing of the interrupt does more
> > > now which includes scheduling which may end up adding to completion
> > > latency.
> >
> > As Chuck noticed, non-AHCI hardware context handlers will benefit.
>
> Maybe I'm off but I'm kinda skeptical that we'd be gaining back the
> overhead we pay by punting to a thread.

Hi Tejun,

As odd as it sounds, I did not mention there is *no* change in IO
performance at all (in my system): neither with one drive nor two.
The change is only about how the interrupt handlers co-exist with
other devices.

I am attaching excerpts from some new perf tests I have done (this
time in legacy interrupt mode). As you can notice, ahci_interrupt()
CPU time drops from 4% to none.

As of your concern wrt threaded handler invocation overhead - I am
not quite sure here, but if SCHED_FIFO policy (the handler runs with)
makes the difference? Anyway, as said above the overall IO does not
suffer.

> --
> tejun

--
Regards,
Alexander Gordeev
[email protected]


2014-10-01 15:31:15

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

On Wed, Oct 01, 2014 at 04:31:14PM +0100, Alexander Gordeev wrote:
> I am attaching excerpts from some new perf tests I have done (this

Attaching :)

--
Regards,
Alexander Gordeev
[email protected]


Attachments:
(No filename) (202.00 B)
perf.hist.0-3.16.0.txt (1.25 kB)
perf.hist.0-3.16.0.opt-intr.txt (771.00 B)
Download all attachments

2014-10-05 02:23:17

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

Hey, Alexander.

On Wed, Oct 01, 2014 at 04:31:15PM +0100, Alexander Gordeev wrote:
> As of your concern wrt threaded handler invocation overhead - I am
> not quite sure here, but if SCHED_FIFO policy (the handler runs with)
> makes the difference? Anyway, as said above the overall IO does not
> suffer.

Hmmm.... so, AFAICS, there's no real pros or cons of going either way,
right? The only thing which could be different is possibly slightly
lower latency in servicing other IRQs or RT tasks on the same CPU but
given that the ahci IRQ handler already doesn't do anything which
takes time, I'm doubtful whether that'd be anything measureable.

I just don't get why ahci bothers with threaded irq, MMSI or not.

Thanks.

--
tejun

2014-10-05 16:16:53

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

A bit of addition.

On Sat, Oct 04, 2014 at 10:23:11PM -0400, Tejun Heo wrote:
> Hmmm.... so, AFAICS, there's no real pros or cons of going either way,
> right? The only thing which could be different is possibly slightly
> lower latency in servicing other IRQs or RT tasks on the same CPU but
> given that the ahci IRQ handler already doesn't do anything which
> takes time, I'm doubtful whether that'd be anything measureable.
>
> I just don't get why ahci bothers with threaded irq, MMSI or not.

I think the thing which bothers me is that due to softirq we end up
bouncing the context twice. IRQ schedules threaded IRQ handler after
doing minimal amount of work. The threaded IRQ handler gets scheduled
and again it doesn't do much but basically just schedules block
softirq to actually run completions which is the heavier part.
Apparently this doesn't seem to hurt measureably but it's just weird.
Why are we bouncing the context twice?

Thanks.

--
tejun

2014-10-06 07:18:34

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

On Sun, Oct 05, 2014 at 12:16:46PM -0400, Tejun Heo wrote:
> I think the thing which bothers me is that due to softirq we end up
> bouncing the context twice. IRQ schedules threaded IRQ handler after
> doing minimal amount of work. The threaded IRQ handler gets scheduled
> and again it doesn't do much but basically just schedules block
> softirq to actually run completions which is the heavier part.
> Apparently this doesn't seem to hurt measureably but it's just weird.

Hi Tejun,

That is exactly the point I was concerned with when stated in one of
changelogs "The downside of this change is introduction of a kernel
thread". Splitting the service routine in two parts is a small change
(in terms of code familiarity). Yet it right away provides benefits I
could observe and justify (to myself at least).

> Why are we bouncing the context twice?

I *did* consider moving the threaded handler code to the softirq part.
I just wanted to get updates in stages: to address hardware interrupts
latency first and possibly threaded hander next. Getting done these two
together would be too big change for me ;)

> --
> tejun

--
Regards,
Alexander Gordeev
[email protected]

2014-10-06 12:58:23

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

Hello, Alexander.

On Mon, Oct 06, 2014 at 08:27:11AM +0100, Alexander Gordeev wrote:
> > Why are we bouncing the context twice?
>
> I *did* consider moving the threaded handler code to the softirq part.
> I just wanted to get updates in stages: to address hardware interrupts
> latency first and possibly threaded hander next. Getting done these two
> together would be too big change for me ;)

I don't think we'd be able to move libata handling to block softirq
and probably end up just doing it from the irq context. Anyways, as
long as you're gonna keep working on it, I have no objection to the
proposed changes. Do you have a refreshed version or is the current
version good for inclusion?

Thanks.

--
tejun

2014-10-06 13:16:08

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

On Mon, Oct 06, 2014 at 08:58:17AM -0400, Tejun Heo wrote:
> I don't think we'd be able to move libata handling to block softirq
> and probably end up just doing it from the irq context. Anyways, as
> long as you're gonna keep working on it, I have no objection to the
> proposed changes. Do you have a refreshed version or is the current
> version good for inclusion?

No, this one would not apply. I can send updated version on top of
v5 I posted earlier. Should I?

> tejun

--
Regards,
Alexander Gordeev
[email protected]

2014-10-06 14:54:09

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 5/6] AHCI: Optimize single IRQ interrupt processing

On Mon, Oct 06, 2014 at 02:24:46PM +0100, Alexander Gordeev wrote:
> On Mon, Oct 06, 2014 at 08:58:17AM -0400, Tejun Heo wrote:
> > I don't think we'd be able to move libata handling to block softirq
> > and probably end up just doing it from the irq context. Anyways, as
> > long as you're gonna keep working on it, I have no objection to the
> > proposed changes. Do you have a refreshed version or is the current
> > version good for inclusion?
>
> No, this one would not apply. I can send updated version on top of
> v5 I posted earlier. Should I?

Yeap, please do so.

Thanks a lot for your patience! :)

--
tejun