Re: [PATCH 0/7] ide: locking improvements

Re: [PATCH 0/7] ide: locking improvements

[Robert Hancock]

Bartlomiej Zolnierkiewicz wrote:
>> Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
>> for something that should essentially be a stable code base in
>> maintenance mode, and now scalability improvements?
> 
> It is the stable code but being in "maintenance only mode" has never
> been true and as long as there are active users & developers there is
> really no reason to change it.

Are there really many active users at this point? I'm not aware of any 
new distributions that are using it. The only people I can see that 
might still want to be using it would be people with old setups or old 
embedded devices.. many of those wouldn't be using newer kernels anyway.

These kinds of changes only will really help scalability on multi-core 
machines which are unlikely to be using this code anyway.. They seem 
rather like putting makeup on a corpse to me..

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Saturday 11 October 2008, Robert Hancock wrote:
> Bartlomiej Zolnierkiewicz wrote:
> >> Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
> >> for something that should essentially be a stable code base in
> >> maintenance mode, and now scalability improvements?
> > 
> > It is the stable code but being in "maintenance only mode" has never
> > been true and as long as there are active users & developers there is
> > really no reason to change it.
> 
> Are there really many active users at this point? I'm not aware of any 
> new distributions that are using it. The only people I can see that 
> might still want to be using it would be people with old setups or old 
> embedded devices.. many of those wouldn't be using newer kernels anyway.

Like I said before: as long as there are any active users/developers
there is no real reason to stop IDE improvements (especially since there
is no complete replacement available).

I also wouldn't worry that much about what some distros are doing.  They
are free to make their own decisions based on whatever criteria they like.

> These kinds of changes only will really help scalability on multi-core 
> machines which are unlikely to be using this code anyway.. They seem 

>From my perspective the main gain of these patches is the increased
maintainability and sanity of the code, scalability improvements are
just an added bonus.

> rather like putting makeup on a corpse to me..

Please refrain from such comments.  Not only the metaphor is completely
bogus but it may sound disrespectful to some IDE developers.

Thanks,
Bart

Re: [PATCH 0/7] ide: locking improvements

[Borislav Petkov]

Hi,

On Sat, Oct 11, 2008 at 01:39:44PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 11 October 2008, Robert Hancock wrote:
> > Bartlomiej Zolnierkiewicz wrote:
> > >> Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
> > >> for something that should essentially be a stable code base in
> > >> maintenance mode, and now scalability improvements?
> > > 
> > > It is the stable code but being in "maintenance only mode" has never
> > > been true and as long as there are active users & developers there is
> > > really no reason to change it.
> > 
> > Are there really many active users at this point? I'm not aware of any 
> > new distributions that are using it. The only people I can see that 
> > might still want to be using it would be people with old setups or old 
> > embedded devices.. many of those wouldn't be using newer kernels anyway.
> 
> Like I said before: as long as there are any active users/developers
> there is no real reason to stop IDE improvements (especially since there
> is no complete replacement available).

... and to be completely clear on things, what Bart and other guys are doing
_is_ maintenance - simply keeping the codebase from becoming a big stinking pile
of sh*t which noone can maintain with time. If you do the effort and count what
percentage of the patches have "There should be no functional change resulting
from this patch" in them you'll see that this is the majority and they rather
clean up/simplify/fix code than add anything new, not even mentioning new
features. So yes, this _is_ maintenance on a larger scale and this is a good(tm)
thing.

> I also wouldn't worry that much about what some distros are doing.  They
> are free to make their own decisions based on whatever criteria they like.
> 
> > These kinds of changes only will really help scalability on multi-core 
> > machines which are unlikely to be using this code anyway.. They seem 
> 
> >From my perspective the main gain of these patches is the increased
> maintainability and sanity of the code, scalability improvements are
> just an added bonus.

and better code/improved scalability is a bad thing because... ?!

> > rather like putting makeup on a corpse to me..

so _NOT_ true.

-- 
Regards/Gruss,
    Boris.

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Sat, Oct 11 2008, Borislav Petkov wrote:
> > >From my perspective the main gain of these patches is the increased
> > maintainability and sanity of the code, scalability improvements are
> > just an added bonus.
> 
> and better code/improved scalability is a bad thing because... ?!

It's a bad thing because nobody on earth cares about IDE scalability,
from a performance POV a modern SATA controller is just better on
several levels. I don't think anybody cares about IDE scaling on 8-16
cores or more, simply because NOBODY is using IDE on such systems.

As such, trying to improve locking is a pointless exercise. And that is
a bad thing, because code change invariably brings in code bugs. Then
see previous mail on lack of coverage testing, and it can naturally be
harmful.

> > > rather like putting makeup on a corpse to me..
> 
> so _NOT_ true.

Depends on what you think is the corpse. Since IDE is essentially dead
and frozen, it IS a corpse and the phrase is then very appropriate. This
is not a personal jab at the IDE guys and does not reflect on the
(mostly) good work they do, just a reflection on the state of IDE in
general.

-- 
Jens Axboe

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Saturday 11 October 2008, Jens Axboe wrote:
> On Sat, Oct 11 2008, Borislav Petkov wrote:
> > > >From my perspective the main gain of these patches is the increased
> > > maintainability and sanity of the code, scalability improvements are
> > > just an added bonus.
> > 
> > and better code/improved scalability is a bad thing because... ?!
> 
> It's a bad thing because nobody on earth cares about IDE scalability,

JFYI: just yesterday I got mail proving otherwise. ;)

> from a performance POV a modern SATA controller is just better on
> several levels. I don't think anybody cares about IDE scaling on 8-16
> cores or more, simply because NOBODY is using IDE on such systems.
> 
> As such, trying to improve locking is a pointless exercise. And that is
> a bad thing, because code change invariably brings in code bugs. Then
> see previous mail on lack of coverage testing, and it can naturally be
> harmful.

Your concerns were already addressed in my reply but I worry that having
a discussion based on technical arguments is not your goal.

Just to repeat: these patches are not hardware specific and obviously
they are not going to be merged today, tomorrow or in a week (they are
2.6.29 material after months of time in pata tree / linux-next).

> > > > rather like putting makeup on a corpse to me..
> > 
> > so _NOT_ true.
> 
> Depends on what you think is the corpse. Since IDE is essentially dead
> and frozen, it IS a corpse and the phrase is then very appropriate. This
> is not a personal jab at the IDE guys and does not reflect on the
> (mostly) good work they do, just a reflection on the state of IDE in
> general.

Interesting statement given that i.e. diffstat-wise pata tree has more
than twice as much stuff queued up for 2.6.28 than "some other" trees
(and we have history of being a _very_ conservative w.r.t. to needlessly
moving code around in drivers/ide/).

Please stop being silly and pushing your view/idea on what other people
should be doing (not to mention ignoring real facts).

Thanks,
Bart

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 11 October 2008, Jens Axboe wrote:
> > On Sat, Oct 11 2008, Borislav Petkov wrote:
> > > > >From my perspective the main gain of these patches is the increased
> > > > maintainability and sanity of the code, scalability improvements are
> > > > just an added bonus.
> > > 
> > > and better code/improved scalability is a bad thing because... ?!
> > 
> > It's a bad thing because nobody on earth cares about IDE scalability,
> 
> JFYI: just yesterday I got mail proving otherwise. ;)

Well, there are lots of crazy people in the world, a request from
someone doesn't necessarily make it a good idea :-)

> > from a performance POV a modern SATA controller is just better on
> > several levels. I don't think anybody cares about IDE scaling on 8-16
> > cores or more, simply because NOBODY is using IDE on such systems.
> > 
> > As such, trying to improve locking is a pointless exercise. And that is
> > a bad thing, because code change invariably brings in code bugs. Then
> > see previous mail on lack of coverage testing, and it can naturally be
> > harmful.
> 
> Your concerns were already addressed in my reply but I worry that having
> a discussion based on technical arguments is not your goal.

You make it sound like I have an alterior motive, which I definitely do
not. I just wondered what all the IDE churn was supposed to be good
for...

> Just to repeat: these patches are not hardware specific and obviously
> they are not going to be merged today, tomorrow or in a week (they are
> 2.6.29 material after months of time in pata tree / linux-next).

It's less about this specific patchset than in general. The specific one
looked fine by itself, it's just the path to to 'IDE lock scaling' that
is a bit crazy to me. Moving IDE to the softirq completion like SCSI
would be a better start, imho. IDE still does most of the processing
under its ide_lock, which isn't even necessary. Making the locking more
finely grained is what I think is pretty crazy.

> > > > > rather like putting makeup on a corpse to me..
> > > 
> > > so _NOT_ true.
> > 
> > Depends on what you think is the corpse. Since IDE is essentially dead
> > and frozen, it IS a corpse and the phrase is then very appropriate. This
> > is not a personal jab at the IDE guys and does not reflect on the
> > (mostly) good work they do, just a reflection on the state of IDE in
> > general.
> 
> Interesting statement given that i.e. diffstat-wise pata tree has more
> than twice as much stuff queued up for 2.6.28 than "some other" trees
> (and we have history of being a _very_ conservative w.r.t. to needlessly
> moving code around in drivers/ide/).
> 
> Please stop being silly and pushing your view/idea on what other people
> should be doing (not to mention ignoring real facts).

Please start by actually _reading_ what I write. Your reply is based on
the code base, my statement pertains to IDE on the hardware side
(devices, controllers, etc). On the scalability front, what new hardware
do you envision shipping with IDE controllers that are actually used for
pushing large amounts of data? People would have to be borderline insane
to make such new hw today.

I'm not pushing my views on anyone, but I am free to share what I
actually think. I actually care about old code stability, hence my
concern with the amount of IDE changes.

-- 
Jens Axboe

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Saturday 11 October 2008, Jens Axboe wrote:
> On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > On Saturday 11 October 2008, Jens Axboe wrote:
> > > On Sat, Oct 11 2008, Borislav Petkov wrote:
> > > > > >From my perspective the main gain of these patches is the increased
> > > > > maintainability and sanity of the code, scalability improvements are
> > > > > just an added bonus.
> > > > 
> > > > and better code/improved scalability is a bad thing because... ?!
> > > 
> > > It's a bad thing because nobody on earth cares about IDE scalability,
> > 
> > JFYI: just yesterday I got mail proving otherwise. ;)
> 
> Well, there are lots of crazy people in the world, a request from
> someone doesn't necessarily make it a good idea :-)
> 
> > > from a performance POV a modern SATA controller is just better on
> > > several levels. I don't think anybody cares about IDE scaling on 8-16
> > > cores or more, simply because NOBODY is using IDE on such systems.
> > > 
> > > As such, trying to improve locking is a pointless exercise. And that is
> > > a bad thing, because code change invariably brings in code bugs. Then
> > > see previous mail on lack of coverage testing, and it can naturally be
> > > harmful.
> > 
> > Your concerns were already addressed in my reply but I worry that having
> > a discussion based on technical arguments is not your goal.
> 
> You make it sound like I have an alterior motive, which I definitely do
> not. I just wondered what all the IDE churn was supposed to be good
> for...
> 
> > Just to repeat: these patches are not hardware specific and obviously
> > they are not going to be merged today, tomorrow or in a week (they are
> > 2.6.29 material after months of time in pata tree / linux-next).
> 
> It's less about this specific patchset than in general. The specific one
> looked fine by itself, it's just the path to to 'IDE lock scaling' that
> is a bit crazy to me. Moving IDE to the softirq completion like SCSI
> would be a better start, imho. IDE still does most of the processing

We are getting there but this can't be done without fixing some other
stuff (like the patch posted today to not process the next request from
hard-IRQ context).  Any help would be much appreciated.

> under its ide_lock, which isn't even necessary. Making the locking more

Well, actually it doesn't do most work under ide_lock (never has been)
as the main means of protection is hwgroup->busy (which is protected by
ide_lock).

[ OTOH some changes in this patchset were inspired by _your_ comments about
  "room for locking improvements" in ide-io.c (IIRC) so kudos to you! ]

> finely grained is what I think is pretty crazy.

The more fine grained locking changes that were posted in separate patch
were first done 3 years ago by Scalex86 guys and they were extensively
tested on Intel hardware (however since other host drivers and things
like IDE settings were abusing ide_lock applying this change back than
was impossible - all of these stuff is fixed in the current Linus' tree).

Sorry for not explaining this clearly in the changelog.

> > > > > > rather like putting makeup on a corpse to me..
> > > > 
> > > > so _NOT_ true.
> > > 
> > > Depends on what you think is the corpse. Since IDE is essentially dead
> > > and frozen, it IS a corpse and the phrase is then very appropriate. This
> > > is not a personal jab at the IDE guys and does not reflect on the
> > > (mostly) good work they do, just a reflection on the state of IDE in
> > > general.
> > 
> > Interesting statement given that i.e. diffstat-wise pata tree has more
> > than twice as much stuff queued up for 2.6.28 than "some other" trees
> > (and we have history of being a _very_ conservative w.r.t. to needlessly
> > moving code around in drivers/ide/).
> > 
> > Please stop being silly and pushing your view/idea on what other people
> > should be doing (not to mention ignoring real facts).
> 
> Please start by actually _reading_ what I write. Your reply is based on
> the code base, my statement pertains to IDE on the hardware side
> (devices, controllers, etc). On the scalability front, what new hardware
> do you envision shipping with IDE controllers that are actually used for
> pushing large amounts of data? People would have to be borderline insane
> to make such new hw today.

Please understand that we are not doing new-hardware-driven-development
here but rather a big maintainance project.  Like I said in reply to Robert
the scalability improvements are an added bonus from my POV -> the main
goal is making the code simpler, harder to abuse and easier to maintain.

> I'm not pushing my views on anyone, but I am free to share what I
> actually think. I actually care about old code stability, hence my
> concern with the amount of IDE changes.

The actual users' feedback about amount of IDE changes have been mostly
positive (some ask for even more changes :) so I don't think that it
should be a reason of a great worries.  I hope that all other concerns
have been also cleared now.

Thanks,
Bart

Re: [PATCH 0/7] ide: locking improvements

[Borislav Petkov]

On Sat, Oct 11, 2008 at 05:56:37PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 11 October 2008, Jens Axboe wrote:
> > On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > > On Saturday 11 October 2008, Jens Axboe wrote:
> > > > On Sat, Oct 11 2008, Borislav Petkov wrote:

[... ]

Don't you just love it when all gets resolved peacefully and everything is fine
and dandy afterwards. I'm so happy we've straightened that one out. Just a final
quick note which by no means is targetting to resparkle the discussion: I have
another bug report, hm, well :), but still, the machine is a 8-core AMD Dell box
(with the option of upgrading to 16 cores) which _is_ using ide-cd so, there are
some people using that code, after all :). I'm sure those people will be happier
when it scales to that many cores.

http://bugzilla.kernel.org/show_bug.cgi?id=11498.

-- 
Regards/Gruss,
    Boris.

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Sat, Oct 11 2008, Borislav Petkov wrote:
> On Sat, Oct 11, 2008 at 05:56:37PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Saturday 11 October 2008, Jens Axboe wrote:
> > > On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > > > On Saturday 11 October 2008, Jens Axboe wrote:
> > > > > On Sat, Oct 11 2008, Borislav Petkov wrote:
> 
> [... ]
> 
> Don't you just love it when all gets resolved peacefully and
> everything is fine and dandy afterwards. I'm so happy we've
> straightened that one out. Just a final quick note which by no means
> is targetting to resparkle the discussion: I have another bug report,
> hm, well :), but still, the machine is a 8-core AMD Dell box (with the
> option of upgrading to 16 cores) which _is_ using ide-cd so, there are
> some people using that code, after all :). I'm sure those people will
> be happier when it scales to that many cores.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=11498.

Not to throw a monkey wrench, but I did add 'pushing data' to that IDE
statement. There is of course lots of systems still with a parallel
channel for things like atapi drives. But that is completely parallel to
the initial argument, those don't care a rats ass about scalability.
They care about it working, that is all.

-- 
Jens Axboe

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> On Saturday 11 October 2008, Jens Axboe wrote:
> > On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > > On Saturday 11 October 2008, Jens Axboe wrote:
> > > > On Sat, Oct 11 2008, Borislav Petkov wrote:
> > > > > > >From my perspective the main gain of these patches is the increased
> > > > > > maintainability and sanity of the code, scalability improvements are
> > > > > > just an added bonus.
> > > > > 
> > > > > and better code/improved scalability is a bad thing because... ?!
> > > > 
> > > > It's a bad thing because nobody on earth cares about IDE scalability,
> > > 
> > > JFYI: just yesterday I got mail proving otherwise. ;)
> > 
> > Well, there are lots of crazy people in the world, a request from
> > someone doesn't necessarily make it a good idea :-)
> > 
> > > > from a performance POV a modern SATA controller is just better on
> > > > several levels. I don't think anybody cares about IDE scaling on 8-16
> > > > cores or more, simply because NOBODY is using IDE on such systems.
> > > > 
> > > > As such, trying to improve locking is a pointless exercise. And that is
> > > > a bad thing, because code change invariably brings in code bugs. Then
> > > > see previous mail on lack of coverage testing, and it can naturally be
> > > > harmful.
> > > 
> > > Your concerns were already addressed in my reply but I worry that having
> > > a discussion based on technical arguments is not your goal.
> > 
> > You make it sound like I have an alterior motive, which I definitely do
> > not. I just wondered what all the IDE churn was supposed to be good
> > for...
> > 
> > > Just to repeat: these patches are not hardware specific and obviously
> > > they are not going to be merged today, tomorrow or in a week (they are
> > > 2.6.29 material after months of time in pata tree / linux-next).
> > 
> > It's less about this specific patchset than in general. The specific one
> > looked fine by itself, it's just the path to to 'IDE lock scaling' that
> > is a bit crazy to me. Moving IDE to the softirq completion like SCSI
> > would be a better start, imho. IDE still does most of the processing
> 
> We are getting there but this can't be done without fixing some other
> stuff (like the patch posted today to not process the next request from
> hard-IRQ context).  Any help would be much appreciated.
> 
> > under its ide_lock, which isn't even necessary. Making the locking more
> 
> Well, actually it doesn't do most work under ide_lock (never has been)
> as the main means of protection is hwgroup->busy (which is protected by
> ide_lock).

Yes it does, it does all of IO processing under the ide_lock, where you
only really need the lock for actually putting the last reference to the
request.

> [ OTOH some changes in this patchset were inspired by _your_ comments about
>   "room for locking improvements" in ide-io.c (IIRC) so kudos to you! ]

And that is indeed what that comment was about :-)
There's certainly a way to make that behave a lot more nicely without
splitting the lock. It's more about latency than lock contention.

> > finely grained is what I think is pretty crazy.
> 
> The more fine grained locking changes that were posted in separate patch
> were first done 3 years ago by Scalex86 guys and they were extensively
> tested on Intel hardware (however since other host drivers and things
> like IDE settings were abusing ide_lock applying this change back than
> was impossible - all of these stuff is fixed in the current Linus' tree).
> 
> Sorry for not explaining this clearly in the changelog.

Yeah I did get that reference, I am still questioning the point of
actually doing that.

> > > > > > > rather like putting makeup on a corpse to me..
> > > > > 
> > > > > so _NOT_ true.
> > > > 
> > > > Depends on what you think is the corpse. Since IDE is essentially dead
> > > > and frozen, it IS a corpse and the phrase is then very appropriate. This
> > > > is not a personal jab at the IDE guys and does not reflect on the
> > > > (mostly) good work they do, just a reflection on the state of IDE in
> > > > general.
> > > 
> > > Interesting statement given that i.e. diffstat-wise pata tree has more
> > > than twice as much stuff queued up for 2.6.28 than "some other" trees
> > > (and we have history of being a _very_ conservative w.r.t. to needlessly
> > > moving code around in drivers/ide/).
> > > 
> > > Please stop being silly and pushing your view/idea on what other people
> > > should be doing (not to mention ignoring real facts).
> > 
> > Please start by actually _reading_ what I write. Your reply is based on
> > the code base, my statement pertains to IDE on the hardware side
> > (devices, controllers, etc). On the scalability front, what new hardware
> > do you envision shipping with IDE controllers that are actually used for
> > pushing large amounts of data? People would have to be borderline insane
> > to make such new hw today.
> 
> Please understand that we are not doing new-hardware-driven-development
> here but rather a big maintainance project.  Like I said in reply to Robert
> the scalability improvements are an added bonus from my POV -> the main
> goal is making the code simpler, harder to abuse and easier to maintain.

I do understand that, as I've said all along I'm more concerned with
coverage of testing since a lot of the supported hardware (not just low
level drivers, things like ide-tape) just isn't used a whole lot these
days. Or the last 5 years, even.

> > I'm not pushing my views on anyone, but I am free to share what I
> > actually think. I actually care about old code stability, hence my
> > concern with the amount of IDE changes.
> 
> The actual users' feedback about amount of IDE changes have been mostly
> positive (some ask for even more changes :) so I don't think that it
> should be a reason of a great worries.  I hope that all other concerns
> have been also cleared now.

Heck that's good, I do hope that I'm mostly over-concerned! I'm not
trying to create a problem where there isn't one, mainly looking for
clarity into the situation.

I noticed one ide-tape tested complaining something broke, and it seems
like he was the only one out there actually using current kernels and
testing it. I worry mostly about the changes breaking somebodys distro 3
years down the line, by which point people may have moved on (and the
old code would have worked).

-- 
Jens Axboe

Re: [PATCH 0/7] ide: locking improvements

[Borislav Petkov]

> > The actual users' feedback about amount of IDE changes have been mostly
> > positive (some ask for even more changes :) so I don't think that it
> > should be a reason of a great worries.  I hope that all other concerns
> > have been also cleared now.
> 
> Heck that's good, I do hope that I'm mostly over-concerned! I'm not
> trying to create a problem where there isn't one, mainly looking for
> clarity into the situation.
> 
> I noticed one ide-tape tested complaining something broke, and it seems
> like he was the only one out there actually using current kernels and
> testing it. I worry mostly about the changes breaking somebodys distro 3
> years down the line, by which point people may have moved on (and the
> old code would have worked).

We try to fix those as fast as possible and they become
highest priority. If we're talking about the same thing, aka
801bd32e205ca6ef78dcaf80121f1eccb89b8c1e, this is obviously already fixed.

It's not like everything is fine with the older code either. For example, I'm
staring at traces w.r.t. bug 11581 which somehow broke ide-cd around the 2.6.19
timeframe and this has been broken for that particular user since then. So I
think that what we do now, generally that anyone is doing something about that
code, replying to bug reports and working with users to solve problems is, IMO,
a lot bigger advantage than doing nothing at all. But your concern is also valid
and we're doing our best to avoid such regressions.

-- 
Regards/Gruss,
    Boris.

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Saturday 11 October 2008, Jens Axboe wrote:
> On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > On Saturday 11 October 2008, Jens Axboe wrote:
> > > On Sat, Oct 11 2008, Bartlomiej Zolnierkiewicz wrote:
> > > > On Saturday 11 October 2008, Jens Axboe wrote:
> > > > > On Sat, Oct 11 2008, Borislav Petkov wrote:
> > > > > > > >From my perspective the main gain of these patches is the increased
> > > > > > > maintainability and sanity of the code, scalability improvements are
> > > > > > > just an added bonus.
> > > > > > 
> > > > > > and better code/improved scalability is a bad thing because... ?!
> > > > > 
> > > > > It's a bad thing because nobody on earth cares about IDE scalability,
> > > > 
> > > > JFYI: just yesterday I got mail proving otherwise. ;)
> > > 
> > > Well, there are lots of crazy people in the world, a request from
> > > someone doesn't necessarily make it a good idea :-)
> > > 
> > > > > from a performance POV a modern SATA controller is just better on
> > > > > several levels. I don't think anybody cares about IDE scaling on 8-16
> > > > > cores or more, simply because NOBODY is using IDE on such systems.
> > > > > 
> > > > > As such, trying to improve locking is a pointless exercise. And that is
> > > > > a bad thing, because code change invariably brings in code bugs. Then
> > > > > see previous mail on lack of coverage testing, and it can naturally be
> > > > > harmful.
> > > > 
> > > > Your concerns were already addressed in my reply but I worry that having
> > > > a discussion based on technical arguments is not your goal.
> > > 
> > > You make it sound like I have an alterior motive, which I definitely do
> > > not. I just wondered what all the IDE churn was supposed to be good
> > > for...
> > > 
> > > > Just to repeat: these patches are not hardware specific and obviously
> > > > they are not going to be merged today, tomorrow or in a week (they are
> > > > 2.6.29 material after months of time in pata tree / linux-next).
> > > 
> > > It's less about this specific patchset than in general. The specific one
> > > looked fine by itself, it's just the path to to 'IDE lock scaling' that
> > > is a bit crazy to me. Moving IDE to the softirq completion like SCSI
> > > would be a better start, imho. IDE still does most of the processing
> > 
> > We are getting there but this can't be done without fixing some other
> > stuff (like the patch posted today to not process the next request from
> > hard-IRQ context).  Any help would be much appreciated.
> > 
> > > under its ide_lock, which isn't even necessary. Making the locking more
> > 
> > Well, actually it doesn't do most work under ide_lock (never has been)
> > as the main means of protection is hwgroup->busy (which is protected by
> > ide_lock).
> 
> Yes it does, it does all of IO processing under the ide_lock, where you
> only really need the lock for actually putting the last reference to the
> request.

When it comes to IO processing (block layer level not hardware one) you
are of course right.  I think that this patchset addresses most of it but
it would be great if you could double check and fix the places that I
missed.

> > [ OTOH some changes in this patchset were inspired by _your_ comments about
> >   "room for locking improvements" in ide-io.c (IIRC) so kudos to you! ]
> 
> And that is indeed what that comment was about :-)
> There's certainly a way to make that behave a lot more nicely without
> splitting the lock. It's more about latency than lock contention.
> 
> > > finely grained is what I think is pretty crazy.
> > 
> > The more fine grained locking changes that were posted in separate patch
> > were first done 3 years ago by Scalex86 guys and they were extensively
> > tested on Intel hardware (however since other host drivers and things
> > like IDE settings were abusing ide_lock applying this change back than
> > was impossible - all of these stuff is fixed in the current Linus' tree).
> > 
> > Sorry for not explaining this clearly in the changelog.
> 
> Yeah I did get that reference, I am still questioning the point of
> actually doing that.

The work has already been done and it is a wortwhile work.  The risk is
quite low (this is the statement based on rather deep understanding of
IDE subsystem, the complete audit of all code-paths affected and all the
testing experiences from Scalex86/me).

Moreover the patch won't be merged after few months of extra testing.

I feel that you still keep on questioning the point of improving IDE
and insist on putting it into "bug-fixes only" mode.  If this is really
the case I'm completely uninterested in discussing it any further.

> > > > > > > > rather like putting makeup on a corpse to me..
> > > > > > 
> > > > > > so _NOT_ true.
> > > > > 
> > > > > Depends on what you think is the corpse. Since IDE is essentially dead
> > > > > and frozen, it IS a corpse and the phrase is then very appropriate. This
> > > > > is not a personal jab at the IDE guys and does not reflect on the
> > > > > (mostly) good work they do, just a reflection on the state of IDE in
> > > > > general.
> > > > 
> > > > Interesting statement given that i.e. diffstat-wise pata tree has more
> > > > than twice as much stuff queued up for 2.6.28 than "some other" trees
> > > > (and we have history of being a _very_ conservative w.r.t. to needlessly
> > > > moving code around in drivers/ide/).
> > > > 
> > > > Please stop being silly and pushing your view/idea on what other people
> > > > should be doing (not to mention ignoring real facts).
> > > 
> > > Please start by actually _reading_ what I write. Your reply is based on
> > > the code base, my statement pertains to IDE on the hardware side
> > > (devices, controllers, etc). On the scalability front, what new hardware
> > > do you envision shipping with IDE controllers that are actually used for
> > > pushing large amounts of data? People would have to be borderline insane
> > > to make such new hw today.
> > 
> > Please understand that we are not doing new-hardware-driven-development
> > here but rather a big maintainance project.  Like I said in reply to Robert
> > the scalability improvements are an added bonus from my POV -> the main
> > goal is making the code simpler, harder to abuse and easier to maintain.
> 
> I do understand that, as I've said all along I'm more concerned with
> coverage of testing since a lot of the supported hardware (not just low
> level drivers, things like ide-tape) just isn't used a whole lot these
> days. Or the last 5 years, even.
> 
> > > I'm not pushing my views on anyone, but I am free to share what I
> > > actually think. I actually care about old code stability, hence my
> > > concern with the amount of IDE changes.
> > 
> > The actual users' feedback about amount of IDE changes have been mostly
> > positive (some ask for even more changes :) so I don't think that it
> > should be a reason of a great worries.  I hope that all other concerns
> > have been also cleared now.
> 
> Heck that's good, I do hope that I'm mostly over-concerned! I'm not
> trying to create a problem where there isn't one, mainly looking for
> clarity into the situation.
> 
> I noticed one ide-tape tested complaining something broke, and it seems
> like he was the only one out there actually using current kernels and

ide-tape is quite a special case since it is on life support since early
2.6.x days (when I ressurected it from somebody's broken bio changes) and
Borislav/me put quite a lot of work into keeping it alive despite having
real difficulties with finding people willing to test changes (fortunately
things seem to be improving here).

> testing it. I worry mostly about the changes breaking somebodys distro 3
> years down the line, by which point people may have moved on (and the
> old code would have worked).

I'm not quite sure I get it here.

Do you mean that we should be more worried about things like:

http://bugzilla.kernel.org/show_bug.cgi?id=11581

?

Thanks,
Bart

Re: [PATCH 0/7] ide: locking improvements

[Robert Hancock]

Bartlomiej Zolnierkiewicz wrote:
> The work has already been done and it is a wortwhile work.  The risk is
> quite low (this is the statement based on rather deep understanding of
> IDE subsystem, the complete audit of all code-paths affected and all the
> testing experiences from Scalex86/me).
> 
> Moreover the patch won't be merged after few months of extra testing.
> 
> I feel that you still keep on questioning the point of improving IDE
> and insist on putting it into "bug-fixes only" mode.  If this is really
> the case I'm completely uninterested in discussing it any further.

What, exactly, is the point of making more than bug-fix-only changes to 
the IDE code today, when we have libata around which is a much better 
code base to work from? I'm afraid it still escapes me. I don't mean to 
denigrate the work that you and other people working on IDE are doing, 
but can't help but think there would be more productive outlets for it..

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Sunday 12 October 2008, Robert Hancock wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > The work has already been done and it is a wortwhile work.  The risk is
> > quite low (this is the statement based on rather deep understanding of
> > IDE subsystem, the complete audit of all code-paths affected and all the
> > testing experiences from Scalex86/me).
> > 
> > Moreover the patch won't be merged after few months of extra testing.
> > 
> > I feel that you still keep on questioning the point of improving IDE
> > and insist on putting it into "bug-fixes only" mode.  If this is really
> > the case I'm completely uninterested in discussing it any further.
> 
> What, exactly, is the point of making more than bug-fix-only changes to 

Please stop this bug-fix-only nonsense already.

Take a look at the bug #11581.  I posted the link in my reply to Jens
because it is a best example that bug-fix-only mode won't really guarantee
a stable, bug-free code in the long-term.  Many of issues at such level
as driver subsystems happen because of "collateral damage" caused by
changes at the higher level.

[ The fact that #11581 was bisected to Jens' commit is just an additional
  spice.  It is likely that bisection went wrong but with git-bisect you
  are guilty-until-proven-innocent so Jens please (finally) help us with
  resolving it. ]

Additionally with open-source projects you have to keep a certain level
of developers' interests because otherwise everybody will be bored to
death and go away work on some other things (unless of course they are
paid to actually work on bugfixes).  Which in turn will result in less
people reviewing changes or doing bugfixes.

IOW in the long-term bug-fix-only code will result in less stable code.

> the IDE code today, when we have libata around which is a much better 
> code base to work from? I'm afraid it still escapes me. I don't mean to 

Simply:

* Not all hardware is supported by libata.

* Today's IDE code is not so different from libata's.

* I'm much more familiar with IDE's code than libata's. :)

> denigrate the work that you and other people working on IDE are doing, 
> but can't help but think there would be more productive outlets for it..

I don't really care.  I work on IDE because it is _fun_.

Thanks,
Bart

[PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

Locking improvements in preparation for replacing the global ide_lock
spinlock by per-hwgroup spinlocks [1].

[1] patch (which is partially based on 2005 patch from Scalex86) for this
is also ready but it needs some more audit and testing

diffstat:
 drivers/ide/ide-cd.c     |   38 ++++++-------
 drivers/ide/ide-io.c     |  129 ++++++++++++++++++++---------------------------
 drivers/ide/ide-ioctls.c |    3 -
 drivers/ide/ide-lib.c    |    7 --
 drivers/ide/ide-proc.c   |   25 +--------
 drivers/ide/ide.c        |    7 --
 6 files changed, 80 insertions(+), 129 deletions(-)

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Wed, Oct 08 2008, Bartlomiej Zolnierkiewicz wrote:
> 
> Locking improvements in preparation for replacing the global ide_lock
> spinlock by per-hwgroup spinlocks [1].
> 
> [1] patch (which is partially based on 2005 patch from Scalex86) for this
> is also ready but it needs some more audit and testing
> 
> diffstat:
>  drivers/ide/ide-cd.c     |   38 ++++++-------
>  drivers/ide/ide-io.c     |  129 ++++++++++++++++++++---------------------------
>  drivers/ide/ide-ioctls.c |    3 -
>  drivers/ide/ide-lib.c    |    7 --
>  drivers/ide/ide-proc.c   |   25 +--------
>  drivers/ide/ide.c        |    7 --
>  6 files changed, 80 insertions(+), 129 deletions(-)

Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
for something that should essentially be a stable code base in
maintenance mode, and now scalability improvements?

Just doesn't make ANY sense to me, sorry. We may end up with a cleaner
code base, but likely also a buggier one. It's not like hardware
coverage testing is all that great, considering some of the ancient
stuff it supports :-)

-- 
Jens Axboe

Re: [PATCH 0/7] ide: locking improvements

[Bartlomiej Zolnierkiewicz]

On Thu, Oct 9, 2008 at 8:51 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Wed, Oct 08 2008, Bartlomiej Zolnierkiewicz wrote:
>>
>> Locking improvements in preparation for replacing the global ide_lock
>> spinlock by per-hwgroup spinlocks [1].
>>
>> [1] patch (which is partially based on 2005 patch from Scalex86) for this
>> is also ready but it needs some more audit and testing
>>
>> diffstat:
>>  drivers/ide/ide-cd.c     |   38 ++++++-------
>>  drivers/ide/ide-io.c     |  129 ++++++++++++++++++++---------------------------
>>  drivers/ide/ide-ioctls.c |    3 -
>>  drivers/ide/ide-lib.c    |    7 --
>>  drivers/ide/ide-proc.c   |   25 +--------
>>  drivers/ide/ide.c        |    7 --
>>  6 files changed, 80 insertions(+), 129 deletions(-)
>
> Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
> for something that should essentially be a stable code base in
> maintenance mode, and now scalability improvements?

It is the stable code but being in "maintenance only mode" has never
been true and as long as there are active users & developers there is
really no reason to change it.

> Just doesn't make ANY sense to me, sorry. We may end up with a cleaner
> code base, but likely also a buggier one. It's not like hardware
> coverage testing is all that great, considering some of the ancient
> stuff it supports :-)

The changes above are relatively safe/simple and are not hardware specific.

Thanks for worring about IDE but we should be fine. :)

Bart

Re: [PATCH 0/7] ide: locking improvements

[Jens Axboe]

On Thu, Oct 09 2008, Bartlomiej Zolnierkiewicz wrote:
> On Thu, Oct 9, 2008 at 8:51 AM, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Wed, Oct 08 2008, Bartlomiej Zolnierkiewicz wrote:
> >>
> >> Locking improvements in preparation for replacing the global ide_lock
> >> spinlock by per-hwgroup spinlocks [1].
> >>
> >> [1] patch (which is partially based on 2005 patch from Scalex86) for this
> >> is also ready but it needs some more audit and testing
> >>
> >> diffstat:
> >>  drivers/ide/ide-cd.c     |   38 ++++++-------
> >>  drivers/ide/ide-io.c     |  129 ++++++++++++++++++++---------------------------
> >>  drivers/ide/ide-ioctls.c |    3 -
> >>  drivers/ide/ide-lib.c    |    7 --
> >>  drivers/ide/ide-proc.c   |   25 +--------
> >>  drivers/ide/ide.c        |    7 --
> >>  6 files changed, 80 insertions(+), 129 deletions(-)
> >
> > Sorry, but I just have to ask 'why'? IDE is seeing a whole lot of churn
> > for something that should essentially be a stable code base in
> > maintenance mode, and now scalability improvements?
> 
> It is the stable code but being in "maintenance only mode" has never
> been true and as long as there are active users & developers there is
> really no reason to change it.

Well, maybe then it's just me who thinks that it definitely SHOULD be in
deep maintenance mode...

> > Just doesn't make ANY sense to me, sorry. We may end up with a cleaner
> > code base, but likely also a buggier one. It's not like hardware
> > coverage testing is all that great, considering some of the ancient
> > stuff it supports :-)
> 
> The changes above are relatively safe/simple and are not hardware specific.
> 
> Thanks for worring about IDE but we should be fine. :)
> 
> Bart

-- 
Jens Axboe