[PATCH] sched.c : correct comment for this_rq_lock() routine

[PATCH] sched.c : correct comment for this_rq_lock() routine

[Robert P. J. Day]

Correct the comment for the this_rq_lock() routine.

Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
---
diff --git a/kernel/sched.c b/kernel/sched.c
index 3399701..94f124e 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -547,7 +547,7 @@ # define schedstat_add(rq, field, amt)	d
 #endif

 /*
- * rq_lock - lock a given runqueue and disable interrupts.
+ * this_rq_lock - lock this runqueue and disable interrupts.
  */
 static inline struct rq *this_rq_lock(void)
 	__acquires(rq->lock)

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[David Rientjes]

On Mon, 30 Oct 2006, Robert P. J. Day wrote:

> 
> Correct the comment for the this_rq_lock() routine.
> 

You submitted this same patch two days ago.

		http://lkml.org/lkml/2006/10/28/54

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Robert P. J. Day]

On Mon, 30 Oct 2006, David Rientjes wrote:

> On Mon, 30 Oct 2006, Robert P. J. Day wrote:
>
> >
> > Correct the comment for the this_rq_lock() routine.
> >
>
> You submitted this same patch two days ago.
>
> 		http://lkml.org/lkml/2006/10/28/54

that's right, i did.  and given that it was a trivial, aesthetic patch
but a couple "git pull" cycles went by without it being applied, i
figured i might as well submit it again.

quite honestly, at this point, given that it's this much trouble to
fix a freaking comment in a single file, i'm seriously losing interest
in wasting any more of my time at this.  life is just too short to
volunteer unpaid labour that just gets dropped on the floor because
you don't know the secret handshake.

rday

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Björn Steinbrink]

On 2006.10.30 16:34:08 -0500, Robert P. J. Day wrote:
> On Mon, 30 Oct 2006, David Rientjes wrote:
> 
> > On Mon, 30 Oct 2006, Robert P. J. Day wrote:
> >
> > >
> > > Correct the comment for the this_rq_lock() routine.
> > >
> >
> > You submitted this same patch two days ago.
> >
> > 		http://lkml.org/lkml/2006/10/28/54
> 
> that's right, i did.  and given that it was a trivial, aesthetic patch
> but a couple "git pull" cycles went by without it being applied, i
> figured i might as well submit it again.
> 
> quite honestly, at this point, given that it's this much trouble to
> fix a freaking comment in a single file, i'm seriously losing interest
> in wasting any more of my time at this.  life is just too short to
> volunteer unpaid labour that just gets dropped on the floor because
> you don't know the secret handshake.

You CC'd trivial@kernel.org, so you've probably read
http://www.kernel.org/pub/linux/kernel/people/bunk/trivial/
which clearly says that it most likely will go into 2.6.20(-rc1). So
your "git pull" tests tell nothing...

I have never submitted a patch to trivial@, so I have no idea if you get
any kind of confirmation that it will be included, but if you get none,
you should probably just assume that it will be included.

Also, you should just take David's reply as a confirmation that your
patch didn't get lost in the noise but was seen by someone. Of course
you should also remember to not re-submit patches that quick, especially
if there is only a weekend in between the two submissions...

Björn

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Borislav Petkov]

On Mon, Oct 30, 2006 at 04:34:08PM -0500, Robert P. J. Day wrote:
> On Mon, 30 Oct 2006, David Rientjes wrote:
> 
> > On Mon, 30 Oct 2006, Robert P. J. Day wrote:
> >
> > >
> > > Correct the comment for the this_rq_lock() routine.
> > >
> >
> > You submitted this same patch two days ago.
> >
> > 		http://lkml.org/lkml/2006/10/28/54
> 
> that's right, i did.  and given that it was a trivial, aesthetic patch
> but a couple "git pull" cycles went by without it being applied, i
> figured i might as well submit it again.
> 
> quite honestly, at this point, given that it's this much trouble to
> fix a freaking comment in a single file, i'm seriously losing interest
> in wasting any more of my time at this.  life is just too short to
> volunteer unpaid labour that just gets dropped on the floor because
> you don't know the secret handshake.

In case you're still doubtful about volunteering:

http://lkml.org/lkml/2004/12/20/255

-- 
Regards/Gruß,
    Boris.

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Robert P. J. Day]

On Tue, 31 Oct 2006, Borislav Petkov wrote:

> In case you're still doubtful about volunteering:
>
> http://lkml.org/lkml/2004/12/20/255

at the risk of being somewhat long-winded, let me explain what i think
is a fundamental conflict involving patch submission, particularly WRT
what are called "trivial" patches.

unlike non-trivial patches such as obvious bug fixes, trivial patches
obviously don't get quite the attention and, because of that, they
have a tendency to collect until someone decides to, say, apply all of
them at once.  most of the time, that's not a problem -- if they're
simply typo fixes or adding comments, then there's no rush.  but
that's not always the case.

in some cases, waiting for the application of one trivial patch might
hold up the submission of another dependent (trivial) patch.  as an
example, i was just poking around the source for the various
"atomic.h" files and noticed a couple possible cleanups:

  1) make sure *everyone* uses "volatile" in the typedef struct (which
	i actually submitted recently)

  2) standardize on "inline", rather than the current messy mixture of
	both "inline" and "__inline__" (which i'm guessing exists for
	backward compatibility)

for the sake of argument, let's assume those were two perfectly
acceptable cleanups and that no one had any objection.  (i'm just
hypothesizing here, so nobody get their underoos in a bunch. :-)

personally, i'd prefer to submit them as separate patches, since i
like my patches to each represent a single logical issue.  so to start
things off, i submit a patch to fix the "volatile" issue.  that
*seems* like a trivial patch, so i submit it as such.  and because
it's marked as trivial, there's no rush to get to it.

if that first trivial patch now changes line numbers sufficiently in
the affected files, then i really have to wait for it to be applied
before i submit the following trivial patch to make sure that second
patch applies cleanly against the updated source.  so i might sit
there, waiting for that first application before i can go ahead with
the second submission.  and i wait, and i wait, and i wait ...

in general, while some patches might very well look "trivial," they
could very well be some preliminary/aesthetic cleanup that has to be
done to set the stage for more involved work that has a real effect.
but that more involved work is just going to have to sit and wait.  if
the trivial stuff got applied more quickly, it would seem that that
would speed up the process of getting to the good stuff.

also, it's never clear whether a patch is going to be appropriate.
someone might spend time creating a patch, only to have it rejected
for reasons they never considered.  it might be more useful to be able
to ask, "hey, i have an idea for a patch, what do you think?", and at
least get some feedback right away.

instead, it seems that, whenever one makes a suggestion along those
lines, the standard reply is just, "submit a patch."  so one creates a
patch and submits it, and it gets rejected after all.  that gets a
little discouraging after a while.

if anyone has a suggestion as to how to make this process more
enjoyable and productive, i'm all ears.

rday

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Nick Piggin]

Robert P. J. Day wrote:

>example, i was just poking around the source for the various
>"atomic.h" files and noticed a couple possible cleanups:
>
>  1) make sure *everyone* uses "volatile" in the typedef struct (which
>	i actually submitted recently)
>

I don't see why. There is nothing in atomic (eg. atomic_read) that says
there must be a compiler barrier around the operation.

Have you checked that the architecture implementation actually needs the
volatile where you've added it?

--

Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Robert P. J. Day]

On Wed, 1 Nov 2006, Nick Piggin wrote:

> Robert P. J. Day wrote:
>
> > example, i was just poking around the source for the various
> > "atomic.h" files and noticed a couple possible cleanups:
> >
> >  1) make sure *everyone* uses "volatile" in the typedef struct (which
> > 	i actually submitted recently)
> >
>
> I don't see why. There is nothing in atomic (eg. atomic_read) that
> says there must be a compiler barrier around the operation.
>
> Have you checked that the architecture implementation actually needs
> the volatile where you've added it?

as just one example, you can read in include/asm-alpha/atomic.h:

/*
 * Counter is volatile to make sure gcc doesn't try to be clever
 * and move things around on us. We need to use _exactly_ the address
 * the user gave us, not some alias that contains the same information.
 */

now it may be that *some* architectures don't specifically require a
volatile counter but, AFAIK, it doesn't actually hurt if it isn't
necessary.  OTOH, if it isn't necessary *at all* for *any*
architecture, then that storage class should be *removed* in its
entirety.

in any event, all this is is another example of what appears to be
niggling and unnecessary differences between arch-specific header
files that could easily be turned into a single, standard definition
that would work for everyone with very little effort (and perhaps some
day be included from a single generic header file to avoid all that
duplication in the first place).

rday

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Nick Piggin]

Robert P. J. Day wrote:
> On Wed, 1 Nov 2006, Nick Piggin wrote:
> 
> 
>>Robert P. J. Day wrote:
>>
>>
>>>example, i was just poking around the source for the various
>>>"atomic.h" files and noticed a couple possible cleanups:
>>>
>>> 1) make sure *everyone* uses "volatile" in the typedef struct (which
>>>	i actually submitted recently)
>>>
>>
>>I don't see why. There is nothing in atomic (eg. atomic_read) that
>>says there must be a compiler barrier around the operation.
>>
>>Have you checked that the architecture implementation actually needs
>>the volatile where you've added it?
> 
> 
> as just one example, you can read in include/asm-alpha/atomic.h:
> 
> /*
>  * Counter is volatile to make sure gcc doesn't try to be clever
>  * and move things around on us. We need to use _exactly_ the address
>  * the user gave us, not some alias that contains the same information.
>  */

asm-alpha has volatile.

> 
> now it may be that *some* architectures don't specifically require a
> volatile counter but, AFAIK, it doesn't actually hurt if it isn't
> necessary.  OTOH, if it isn't necessary *at all* for *any*
> architecture, then that storage class should be *removed* in its
> entirety.

Then given that architecture specific code is private to that architecture,
the logical conclusion is that if it isn't required by *an* architectures,
then it should be removed as well.

IOW, you can't assume that because one arch does something one way, that
another has the same restrictions.

> in any event, all this is is another example of what appears to be
> niggling and unnecessary differences between arch-specific header
> files that could easily be turned into a single, standard definition
> that would work for everyone with very little effort (and perhaps some
> day be included from a single generic header file to avoid all that
> duplication in the first place).

So if you want to do that, then I'd prefer that you instead go through all
arch headers and figure out where the volatile is needed, and why, and
document it.

For example FRV, at a glance, doesn't need volatile AFAIKS. Probably neither
do a lot of architectures, given that atomic_read/set/add/etc. need not
imply compiler or memory barriers (I think).

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [PATCH] sched.c : correct comment for this_rq_lock() routine

[Borislav Petkov]

On Tue, Oct 31, 2006 at 01:11:06PM -0500, Robert P. J. Day wrote:
> On Tue, 31 Oct 2006, Borislav Petkov wrote:
> 
> > In case you're still doubtful about volunteering:
> >
> > http://lkml.org/lkml/2004/12/20/255
> 
> at the risk of being somewhat long-winded, let me explain what i think
> is a fundamental conflict involving patch submission, particularly WRT
> what are called "trivial" patches.

These are all sound arguments but I guess there's litte one can do in a
community development process where every other developer/maintainer has a
different way of classifying and sorting code commits in the order of their
precedence/importance...

> unlike non-trivial patches such as obvious bug fixes, trivial patches
> obviously don't get quite the attention and, because of that, they
> have a tendency to collect until someone decides to, say, apply all of
> them at once.  most of the time, that's not a problem -- if they're
> simply typo fixes or adding comments, then there's no rush.  but
> that's not always the case.
> 
> in some cases, waiting for the application of one trivial patch might
> hold up the submission of another dependent (trivial) patch.  as an
> example, i was just poking around the source for the various
> "atomic.h" files and noticed a couple possible cleanups:
> 
>   1) make sure *everyone* uses "volatile" in the typedef struct (which
> 	i actually submitted recently)
> 
>   2) standardize on "inline", rather than the current messy mixture of
> 	both "inline" and "__inline__" (which i'm guessing exists for
> 	backward compatibility)
> 
> for the sake of argument, let's assume those were two perfectly
> acceptable cleanups and that no one had any objection.  (i'm just
> hypothesizing here, so nobody get their underoos in a bunch. :-)
> 
> personally, i'd prefer to submit them as separate patches, since i
> like my patches to each represent a single logical issue.  so to start
> things off, i submit a patch to fix the "volatile" issue.  that
> *seems* like a trivial patch, so i submit it as such.  and because
> it's marked as trivial, there's no rush to get to it.

... and because of that, the more trivial it is, the faster it becomes obsolete
so when i first read the announcement on the trivial-list that patches submitted
for kernel version 2.x.n are going to be merged probably in version 2.x.n+2 or
something like that, I thought, well, hell, someone has a really nasty desire
for applying patches with fuzziness of <Really Big Number>. Or even worse,
patches which are not even applicable anymore.

But the problem is, IMHO, that the subsystem maintainers are simply swamped
with, let's say with the risk of sounding insensitive, more important work, 
 so that they just don't have the time for the trivial stuff. So, sometimes, 
there can be a situation when a trivial patch gets forgotten but if you try
thinking of an adequate solution for this not happening, you're soon going to
realize that a solution for trivial patches cannot be a trivial one:). Hell, I
don't even think that there will be a sufficient one.

<snip/>
-- 
Regards/Gruß,
    Boris.