RE: PATCH: Replace current->state with set_current_state in 2.5.6 8

RE: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Perez-Gonzalez, Inaky]

> From: Gabriel Devenyi [mailto:devenyga@mcmaster.ca]
> 
> This patch appies to 2.5.68 and replaces any remaining current->state
lines
>  with set_current_state. This from the TODO list of Kernel Janitors.
> 
>
http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-set_current_state.patch

Some time ago I sent a patch doing this only on */fs/* [not the filesystem's
code, just the common stuff]. It was dismissed by Linus under
I-don't-know-what
-the-hell-reasons (it's very smart to dismiss something without reason,
gives
the original poster a very clear idea of what needs to be changed -
nevermind,
just being ironic). 

However, I'd suggest to post this into the Kernel Janitors mailing list and
let one of the big guys there swipe it in.

Maybe Robert Love can provide more highlight.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

Re: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Randy.Dunlap]

On Tue, 6 May 2003 17:33:26 -0700  "Perez-Gonzalez, Inaky" <inaky.perez-gonzalez@intel.com> wrote:

| > From: Gabriel Devenyi [mailto:devenyga@mcmaster.ca]
| > 
| > This patch appies to 2.5.68 and replaces any remaining current->state
| lines
| >  with set_current_state. This from the TODO list of Kernel Janitors.
| > 
| >
| http://muss.mcmaster.ca/~devenyga/patch-linux-2.5.68-set_current_state.patch
| 
| Some time ago I sent a patch doing this only on */fs/* [not the filesystem's
| code, just the common stuff]. It was dismissed by Linus under
| I-don't-know-what
| -the-hell-reasons (it's very smart to dismiss something without reason,
| gives
| the original poster a very clear idea of what needs to be changed -
| nevermind, just being ironic). 
| 
| However, I'd suggest to post this into the Kernel Janitors mailing list and
| let one of the big guys there swipe it in.
| 
| Maybe Robert Love can provide more highlight.


Yes, the KJ list has already seen this patch and commented on some version
of it.

Folks, IMO for KJ work to be successful (merged) and rewarding, and for it
to continue, we need:

- TODO list updated with accurate descriptions of problems and expected
  changes to fix them;
- a plan of attack for getting them merged;

We shouldn't just expect (for example) Gabriel's patch to be merged
on its own.

Even if Gabriel's patch is perfect, I don't expect that Linus will merge it,
esp. not now, but not even earlier in 2.5 days.  For one thing, it's
176 KB, so it needs to be broken down by subsystem/driver/filesystem/etc.

Then it needs some exposure, like living in -ac or -mm or -pick1,
or at least some testing (everyday usage) by a few people, with reports
from them.

And I don't really want to review a 176 KB patch (although I did already
look over most of it a few days ago).  Do people want to take portions
of it for review and then see about Alan merging it, e.g.?

--
~Randy

Re: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Gerrit Huizenga]

On Tue, 06 May 2003 18:24:56 PDT, "Randy.Dunlap" wrote:
> 
> On Tue, 6 May 2003 17:33:26 -0700  "Perez-Gonzalez, Inaky" <inaky.perez-gonzalez@intel.com> wrote:
> 
> | However, I'd suggest to post this into the Kernel Janitors mailing list and
> | let one of the big guys there swipe it in.
> | 
> 
> Yes, the KJ list has already seen this patch and commented on some version
> of it.
 
> Then it needs some exposure, like living in -ac or -mm or -pick1,
> or at least some testing (everyday usage) by a few people, with reports
> from them.
> 
> And I don't really want to review a 176 KB patch (although I did already
> look over most of it a few days ago).  Do people want to take portions
> of it for review and then see about Alan merging it, e.g.?

Hmm.  Has anyone considered a "Kernel Janitor's" tree?  More specifically,
a patch set, much like -ac or -mm, with the current cleanups so they
can be tested, pulled, run through automated batch testing, etc.?

gerrit

Re: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Arnaldo Carvalho de Melo]

Em Tue, May 06, 2003 at 07:01:27PM -0700, Gerrit Huizenga escreveu:
> On Tue, 06 May 2003 18:24:56 PDT, "Randy.Dunlap" wrote:
> > 
> > On Tue, 6 May 2003 17:33:26 -0700  "Perez-Gonzalez, Inaky" <inaky.perez-gonzalez@intel.com> wrote:
> > 
> > | However, I'd suggest to post this into the Kernel Janitors mailing list and
> > | let one of the big guys there swipe it in.
> > | 
> > 
> > Yes, the KJ list has already seen this patch and commented on some version
> > of it.
>  
> > Then it needs some exposure, like living in -ac or -mm or -pick1,
> > or at least some testing (everyday usage) by a few people, with reports
> > from them.
> > 
> > And I don't really want to review a 176 KB patch (although I did already
> > look over most of it a few days ago).  Do people want to take portions
> > of it for review and then see about Alan merging it, e.g.?
> 
> Hmm.  Has anyone considered a "Kernel Janitor's" tree?  More specifically,
> a patch set, much like -ac or -mm, with the current cleanups so they
> can be tested, pulled, run through automated batch testing, etc.?

That is an interesting idea, I'll probably start one.

- Arnaldo

Re: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Paul P Komkoff Jr]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Replying to Randy.Dunlap:
> Even if Gabriel's patch is perfect, I don't expect that Linus will merge it,
> esp. not now, but not even earlier in 2.5 days.  For one thing, it's
> 176 KB, so it needs to be broken down by subsystem/driver/filesystem/etc.

almost one year ago I've did similar work. Even split it in small pieces.
Even encouraged some subsystem maintainers to do some movements in this
direction (greg kh, actually).

Even running my own patched kernel with this patches in on my production
servers for a months without single hang or reboot.

Even have 2.4 bk tree (way outdated, but patches are in). Even can publish it.

> Then it needs some exposure, like living in -ac or -mm or -pick1,
> or at least some testing (everyday usage) by a few people, with reports
> from them.

After a couple of attempts to attend people who make decisions to that issue
I've stopped doing that, just almost like Keith Owens with his kbuild 2.5

> And I don't really want to review a 176 KB patch (although I did already
> look over most of it a few days ago).  Do people want to take portions
> of it for review and then see about Alan merging it, e.g.?

I don't think even sed with simple s/// can misplace set_current_state into
something unrelated (if it is not buggy sed ;)

And with bitkeeper (or other tool that can do colored diff representation)
it will be even simplier.

- -- 
Paul P 'Stingray' Komkoff Jr // http://stingr.net/key <- my pgp key
 This message represents the official view of the voices in my head
-----BEGIN PGP SIGNATURE-----

iD8DBQE+uNNByMW8naS07KQRA52oAKDL1irCb0uRhN1coX/u8FB1q00SQwCcDYvq
FF8b4rCJLJVKtFFmCGTu1Xo=
=M478
-----END PGP SIGNATURE-----

RE: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Perez-Gonzalez, Inaky]

> From: Arnaldo Carvalho de Melo [mailto:acme@conectiva.com.br]
> 
> > > And I don't really want to review a 176 KB patch (although I did
already
> > > look over most of it a few days ago).  Do people want to take portions
> > > of it for review and then see about Alan merging it, e.g.?

As long as they use set_current_state() and not the __ counterpart,
then they are ok [the memory barrier being to blame for the lost
performance if any is found].

> > Hmm.  Has anyone considered a "Kernel Janitor's" tree?  More
specifically,
> > a patch set, much like -ac or -mm, with the current cleanups so they
> > can be tested, pulled, run through automated batch testing, etc.?
> 
> That is an interesting idea, I'll probably start one.

That's very interesting.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

RE: PATCH: Replace current->state with set_current_state in 2.5.6 8

[Randy.Dunlap]

>
>> From: Arnaldo Carvalho de Melo [mailto:acme@conectiva.com.br]
>>
>> > > And I don't really want to review a 176 KB patch (although I did
> already
>> > > look over most of it a few days ago).  Do people want to take portions
>> of it for review and then see about Alan merging it, e.g.?
>
> As long as they use set_current_state() and not the __ counterpart, then
> they are ok [the memory barrier being to blame for the lost
> performance if any is found].

Yes, last week at least the patch did use set_current_state() almost
exclusively, even when it didn't need to -- most likely.

>> > Hmm.  Has anyone considered a "Kernel Janitor's" tree?  More
> specifically,
>> > a patch set, much like -ac or -mm, with the current cleanups so they can
>> be tested, pulled, run through automated batch testing, etc.?
>>
>> That is an interesting idea, I'll probably start one.

Yay!
Thanks, acme.

~Randy