* [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
@ 2008-12-10 18:21 Steven Rostedt
2008-12-10 18:33 ` Eric Dumazet
0 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2008-12-10 18:21 UTC (permalink / raw)
To: LKML; +Cc: Ingo Molnar, Linus Torvalds, Andrew Morton, Arnaldo Carvalho de Melo
The following patch is in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: cleanups
Steven Rostedt (1):
replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
----
net/dccp/proto.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
---------------------------
commit 3e1de819261b68a48b09719694c7e3579a8e4186
Author: Steven Rostedt <srostedt@redhat.com>
Date: Wed Dec 10 12:49:47 2008 -0500
replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
Impact: clean up
RW_LOCK_UNLOCKED has been deprecated and superseded by
__RW_LOCK_UNLOCKED(name). This patch removes one of the last
remaining users of it.
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index d0bd348..e6e54a7 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -45,7 +45,7 @@ atomic_t dccp_orphan_count = ATOMIC_INIT(0);
EXPORT_SYMBOL_GPL(dccp_orphan_count);
struct inet_hashinfo __cacheline_aligned dccp_hashinfo = {
- .lhash_lock = RW_LOCK_UNLOCKED,
+ .lhash_lock = __RW_LOCK_UNLOCKED(dccp_hashinfo.lhash_lock),
.lhash_users = ATOMIC_INIT(0),
.lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(dccp_hashinfo.lhash_wait),
};
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
2008-12-10 18:21 [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c Steven Rostedt
@ 2008-12-10 18:33 ` Eric Dumazet
2008-12-10 19:00 ` Steven Rostedt
2008-12-10 20:06 ` [PATCH] update rwlock initialization for nat_table Steven Rostedt
0 siblings, 2 replies; 12+ messages in thread
From: Eric Dumazet @ 2008-12-10 18:33 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Ingo Molnar, Linus Torvalds, Andrew Morton,
Arnaldo Carvalho de Melo, David S. Miller, Linux Netdev List
cced netdev
Steven Rostedt a écrit :
> The following patch is in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
>
> branch: cleanups
>
>
> Steven Rostedt (1):
> replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
>
> ----
> net/dccp/proto.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> ---------------------------
> commit 3e1de819261b68a48b09719694c7e3579a8e4186
> Author: Steven Rostedt <srostedt@redhat.com>
> Date: Wed Dec 10 12:49:47 2008 -0500
>
> replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
>
> Impact: clean up
>
> RW_LOCK_UNLOCKED has been deprecated and superseded by
> __RW_LOCK_UNLOCKED(name). This patch removes one of the last
> remaining users of it.
>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
>
> diff --git a/net/dccp/proto.c b/net/dccp/proto.c
> index d0bd348..e6e54a7 100644
> --- a/net/dccp/proto.c
> +++ b/net/dccp/proto.c
> @@ -45,7 +45,7 @@ atomic_t dccp_orphan_count = ATOMIC_INIT(0);
> EXPORT_SYMBOL_GPL(dccp_orphan_count);
>
> struct inet_hashinfo __cacheline_aligned dccp_hashinfo = {
> - .lhash_lock = RW_LOCK_UNLOCKED,
> + .lhash_lock = __RW_LOCK_UNLOCKED(dccp_hashinfo.lhash_lock),
> .lhash_users = ATOMIC_INIT(0),
> .lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(dccp_hashinfo.lhash_wait),
> };
> --
Hum... this rwlock doesnt exist anymore on net-next-2.6, all this stuff
was converted to RCU for upcoming 2.6.29
struct inet_hashinfo dccp_hashinfo;
EXPORT_SYMBOL_GPL(dccp_hashinfo);
I guess such a patch wont ease David job when 2.6.29 merge window opens
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
2008-12-10 18:33 ` Eric Dumazet
@ 2008-12-10 19:00 ` Steven Rostedt
2008-12-10 20:06 ` [PATCH] update rwlock initialization for nat_table Steven Rostedt
1 sibling, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2008-12-10 19:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: LKML, Ingo Molnar, Linus Torvalds, Andrew Morton,
Arnaldo Carvalho de Melo, David S. Miller, Linux Netdev List
On Wed, 10 Dec 2008, Eric Dumazet wrote:
> cced netdev
Thanks,
>
> Steven Rostedt a ?crit :
> >
> > struct inet_hashinfo __cacheline_aligned dccp_hashinfo = {
> > - .lhash_lock = RW_LOCK_UNLOCKED,
> > + .lhash_lock = __RW_LOCK_UNLOCKED(dccp_hashinfo.lhash_lock),
> > .lhash_users = ATOMIC_INIT(0),
> > .lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(dccp_hashinfo.lhash_wait),
> > };
> > --
>
> Hum... this rwlock doesnt exist anymore on net-next-2.6, all this stuff
> was converted to RCU for upcoming 2.6.29
>
> struct inet_hashinfo dccp_hashinfo;
> EXPORT_SYMBOL_GPL(dccp_hashinfo);
>
> I guess such a patch wont ease David job when 2.6.29 merge window opens
One of the requirements in the new rt git tree is that all rwlocks must
use the __RW_LOCK_UNLOCK(lock) macro. I had to fix it in our tree, but
whenever I do a clean up patch, I like to share it ;-)
-- Steve
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] update rwlock initialization for nat_table
2008-12-10 18:33 ` Eric Dumazet
2008-12-10 19:00 ` Steven Rostedt
@ 2008-12-10 20:06 ` Steven Rostedt
2008-12-11 22:18 ` Andrew Morton
2008-12-15 8:20 ` David Miller
1 sibling, 2 replies; 12+ messages in thread
From: Steven Rostedt @ 2008-12-10 20:06 UTC (permalink / raw)
To: Linux Netdev List, LKML
Cc: Eric Dumazet, Ingo Molnar, Andrew Morton,
Arnaldo Carvalho de Melo, David S. Miller
The following patch is in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: cleanups
Steven Rostedt (1):
update rwlock initialization for nat_table
----
net/ipv4/netfilter/nf_nat_rule.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
---------------------------
commit d4175059c8f95e4cd58e0efaa85610ca59469fbd
Author: Steven Rostedt <srostedt@redhat.com>
Date: Wed Dec 10 15:00:09 2008 -0500
update rwlock initialization for nat_table
Impact: clean up
The commit e099a173573ce1ba171092aee7bb3c72ea686e59
(netfilter: netns nat: per-netns NAT table) renamed the
nat_table from __nat_table to nat_table without updating the
__RW_LOCK_UNLOCKED(__nat_table.lock).
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
diff --git a/net/ipv4/netfilter/nf_nat_rule.c b/net/ipv4/netfilter/nf_nat_rule.c
index bea54a6..8d489e7 100644
--- a/net/ipv4/netfilter/nf_nat_rule.c
+++ b/net/ipv4/netfilter/nf_nat_rule.c
@@ -61,7 +61,7 @@ static struct
static struct xt_table nat_table = {
.name = "nat",
.valid_hooks = NAT_VALID_HOOKS,
- .lock = __RW_LOCK_UNLOCKED(__nat_table.lock),
+ .lock = __RW_LOCK_UNLOCKED(nat_table.lock),
.me = THIS_MODULE,
.af = AF_INET,
};
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] update rwlock initialization for nat_table
2008-12-10 20:06 ` [PATCH] update rwlock initialization for nat_table Steven Rostedt
@ 2008-12-11 22:18 ` Andrew Morton
2008-12-11 22:24 ` Steven Rostedt
2008-12-15 8:20 ` David Miller
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-12-11 22:18 UTC (permalink / raw)
To: Steven Rostedt; +Cc: netdev, linux-kernel, dada1, mingo, acme, davem
On Wed, 10 Dec 2008 15:06:00 -0500 (EST)
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> The following patch is in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
>
> branch: cleanups
>
>
> Steven Rostedt (1):
> update rwlock initialization for nat_table
>
> ----
> net/ipv4/netfilter/nf_nat_rule.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> ---------------------------
> commit d4175059c8f95e4cd58e0efaa85610ca59469fbd
> Author: Steven Rostedt <srostedt@redhat.com>
> Date: Wed Dec 10 15:00:09 2008 -0500
>
> update rwlock initialization for nat_table
>
> Impact: clean up
It's more than a "cleanup"?
> The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> (netfilter: netns nat: per-netns NAT table) renamed the
> nat_table from __nat_table to nat_table without updating the
> __RW_LOCK_UNLOCKED(__nat_table.lock).
>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
>
> diff --git a/net/ipv4/netfilter/nf_nat_rule.c b/net/ipv4/netfilter/nf_nat_rule.c
> index bea54a6..8d489e7 100644
> --- a/net/ipv4/netfilter/nf_nat_rule.c
> +++ b/net/ipv4/netfilter/nf_nat_rule.c
> @@ -61,7 +61,7 @@ static struct
> static struct xt_table nat_table = {
> .name = "nat",
> .valid_hooks = NAT_VALID_HOOKS,
> - .lock = __RW_LOCK_UNLOCKED(__nat_table.lock),
> + .lock = __RW_LOCK_UNLOCKED(nat_table.lock),
> .me = THIS_MODULE,
> .af = AF_INET,
> };
At present any lockdep messages relating to this lock will print the
wrong name. So it's a nanobug, I think?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] update rwlock initialization for nat_table
2008-12-11 22:18 ` Andrew Morton
@ 2008-12-11 22:24 ` Steven Rostedt
0 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2008-12-11 22:24 UTC (permalink / raw)
To: Andrew Morton; +Cc: netdev, linux-kernel, dada1, mingo, acme, davem
On Thu, 11 Dec 2008, Andrew Morton wrote:
> On Wed, 10 Dec 2008 15:06:00 -0500 (EST)
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> > ---------------------------
> > commit d4175059c8f95e4cd58e0efaa85610ca59469fbd
> > Author: Steven Rostedt <srostedt@redhat.com>
> > Date: Wed Dec 10 15:00:09 2008 -0500
> >
> > update rwlock initialization for nat_table
> >
> > Impact: clean up
>
> It's more than a "cleanup"?
>
> > The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> > (netfilter: netns nat: per-netns NAT table) renamed the
> > nat_table from __nat_table to nat_table without updating the
> > __RW_LOCK_UNLOCKED(__nat_table.lock).
> >
> > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> >
> > diff --git a/net/ipv4/netfilter/nf_nat_rule.c b/net/ipv4/netfilter/nf_nat_rule.c
> > index bea54a6..8d489e7 100644
> > --- a/net/ipv4/netfilter/nf_nat_rule.c
> > +++ b/net/ipv4/netfilter/nf_nat_rule.c
> > @@ -61,7 +61,7 @@ static struct
> > static struct xt_table nat_table = {
> > .name = "nat",
> > .valid_hooks = NAT_VALID_HOOKS,
> > - .lock = __RW_LOCK_UNLOCKED(__nat_table.lock),
> > + .lock = __RW_LOCK_UNLOCKED(nat_table.lock),
> > .me = THIS_MODULE,
> > .af = AF_INET,
> > };
>
> At present any lockdep messages relating to this lock will print the
> wrong name. So it's a nanobug, I think?
Well, I'm now working on the RT git tree, and it has a much stronger
requirement on __RW_LOCK_UNLOCK. It actually uses the variable inside.
For mainline, it is a nanobug, but for -rt it is a compiler error. Since
it is still a clean up, it would be nice to have it in mainline ;-)
-- Steve
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] update rwlock initialization for nat_table
2008-12-10 20:06 ` [PATCH] update rwlock initialization for nat_table Steven Rostedt
2008-12-11 22:18 ` Andrew Morton
@ 2008-12-15 8:20 ` David Miller
2008-12-16 1:10 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Alexey Dobriyan
1 sibling, 1 reply; 12+ messages in thread
From: David Miller @ 2008-12-15 8:20 UTC (permalink / raw)
To: rostedt; +Cc: netdev, linux-kernel, dada1, mingo, akpm, acme
From: Steven Rostedt <rostedt@goodmis.org>
Date: Wed, 10 Dec 2008 15:06:00 -0500 (EST)
> update rwlock initialization for nat_table
>
> Impact: clean up
>
> The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> (netfilter: netns nat: per-netns NAT table) renamed the
> nat_table from __nat_table to nat_table without updating the
> __RW_LOCK_UNLOCKED(__nat_table.lock).
>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
Applied to net-2.6, thanks Steven.
As Andrew mentioned this is a bug (albeit a "nano-bug" as you
called it :-) so I removed the Impact line in the commit
message when applying this.
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
2008-12-15 8:20 ` David Miller
@ 2008-12-16 1:10 ` Alexey Dobriyan
2008-12-16 1:19 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Alexey Dobriyan @ 2008-12-16 1:10 UTC (permalink / raw)
To: David Miller; +Cc: rostedt, linux-kernel, dada1, mingo, akpm, acme
On Mon, Dec 15, 2008 at 12:20:19AM -0800, David Miller wrote:
> > update rwlock initialization for nat_table
> >
> > Impact: clean up
> >
> > The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> > (netfilter: netns nat: per-netns NAT table) renamed the
> > nat_table from __nat_table to nat_table without updating the
> > __RW_LOCK_UNLOCKED(__nat_table.lock).
> >
> > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
>
> Applied to net-2.6, thanks Steven.
>
> As Andrew mentioned this is a bug (albeit a "nano-bug" as you
> called it :-) so I removed the Impact line in the commit
> message when applying this.
Speaking of Impact: lines, is this a new fashion or what?
Looking at the ones which are already in official tree, they are either
trivially duplicating Subject: line, or effectively duplicating Subject: line,
or cover up for insufficiently informative (read: badly written) Subject: line,
or simply useless.
Subject: sched: CPU remove deadlock fix
Impact: fix possible deadlock in CPU hot-remove path
What prevented to write "Subject: sched: fix possible deadlock in CPU hot-remove path"?
AMD IOMMU: __unmap_single: check for bad_dma_address instead of 0
Impact: minor fix
Well...
I have an idea on how to make them remotely useful, but can we agree that there is
a problem arising here?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
2008-12-16 1:10 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Alexey Dobriyan
@ 2008-12-16 1:19 ` Andrew Morton
2008-12-16 11:15 ` Arnaldo Carvalho de Melo
2008-12-16 6:38 ` Impact: David Miller
2008-12-16 23:00 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Ingo Molnar
2 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-12-16 1:19 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: David Miller, rostedt, linux-kernel, dada1, mingo, acme
On Tue, 16 Dec 2008 04:10:39 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Mon, Dec 15, 2008 at 12:20:19AM -0800, David Miller wrote:
> > > update rwlock initialization for nat_table
> > >
> > > Impact: clean up
> > >
> > > The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> > > (netfilter: netns nat: per-netns NAT table) renamed the
> > > nat_table from __nat_table to nat_table without updating the
> > > __RW_LOCK_UNLOCKED(__nat_table.lock).
> > >
> > > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> >
> > Applied to net-2.6, thanks Steven.
> >
> > As Andrew mentioned this is a bug (albeit a "nano-bug" as you
> > called it :-) so I removed the Impact line in the commit
> > message when applying this.
>
> Speaking of Impact: lines, is this a new fashion or what?
>
> Looking at the ones which are already in official tree, they are either
> trivially duplicating Subject: line, or effectively duplicating Subject: line,
> or cover up for insufficiently informative (read: badly written) Subject: line,
> or simply useless.
>
>
> Subject: sched: CPU remove deadlock fix
> Impact: fix possible deadlock in CPU hot-remove path
>
> What prevented to write "Subject: sched: fix possible deadlock in CPU hot-remove path"?
>
>
> AMD IOMMU: __unmap_single: check for bad_dma_address instead of 0
> Impact: minor fix
>
> Well...
>
> I have an idea on how to make them remotely useful, but can we agree that there is
> a problem arising here?
heh, I must say that the ones I've seen haven't been very useful.
However... Given the amount of time I (and others, to a lesser extent)
spend complaining about and scratching heads over crappy changelogs, we
would benefit from having a standard changelog template.
Something which guides people to creating a good changelog. But it
would have to be short, and carefully written. It should learn from
history, to wit:
- ./REPORTING-BUGS has a template and afaik it has never elicited any
useful information.
- Documentation/SubmittingPatches has info on how to write a
changelog, and people blithely ignore it.
- kerneldoc provide a template of sorts, and we see that filling out
templates puts people's brains into "filling out a template" mode,
rather than into "communicating information" mode.
An interesting problem.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Impact:
2008-12-16 1:10 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Alexey Dobriyan
2008-12-16 1:19 ` Andrew Morton
@ 2008-12-16 6:38 ` David Miller
2008-12-16 23:00 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Ingo Molnar
2 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2008-12-16 6:38 UTC (permalink / raw)
To: adobriyan; +Cc: rostedt, linux-kernel, dada1, mingo, akpm, acme
From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Tue, 16 Dec 2008 04:10:39 +0300
> On Mon, Dec 15, 2008 at 12:20:19AM -0800, David Miller wrote:
> > > update rwlock initialization for nat_table
> > >
> > > Impact: clean up
> > >
> > > The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> > > (netfilter: netns nat: per-netns NAT table) renamed the
> > > nat_table from __nat_table to nat_table without updating the
> > > __RW_LOCK_UNLOCKED(__nat_table.lock).
> > >
> > > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> >
> > Applied to net-2.6, thanks Steven.
> >
> > As Andrew mentioned this is a bug (albeit a "nano-bug" as you
> > called it :-) so I removed the Impact line in the commit
> > message when applying this.
>
> Speaking of Impact: lines, is this a new fashion or what?
I don't know what it is, but I find them pointless.
Just provide a proper verbose commit message and be done with it.
At the rate we're going we'll soon have commit messages like:
--------------------
Impact: Crashes kernel
Cause: Race between timer and workqueue
Implementation: ...
--------------------
I mean, spare me...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
2008-12-16 1:19 ` Andrew Morton
@ 2008-12-16 11:15 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2008-12-16 11:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Alexey Dobriyan, David Miller, rostedt, linux-kernel, dada1, mingo
Em Mon, Dec 15, 2008 at 05:19:35PM -0800, Andrew Morton escreveu:
> On Tue, 16 Dec 2008 04:10:39 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > Speaking of Impact: lines, is this a new fashion or what?
> >
> > Looking at the ones which are already in official tree, they are either
> > trivially duplicating Subject: line, or effectively duplicating Subject: line,
> > or cover up for insufficiently informative (read: badly written) Subject: line,
> > or simply useless.
> >
> >
> > Subject: sched: CPU remove deadlock fix
> > Impact: fix possible deadlock in CPU hot-remove path
> >
> > What prevented to write "Subject: sched: fix possible deadlock in CPU hot-remove path"?
> >
> >
> > AMD IOMMU: __unmap_single: check for bad_dma_address instead of 0
> > Impact: minor fix
> >
> > Well...
> >
> > I have an idea on how to make them remotely useful, but can we agree that there is
> > a problem arising here?
>
> heh, I must say that the ones I've seen haven't been very useful.
>
> However... Given the amount of time I (and others, to a lesser extent)
> spend complaining about and scratching heads over crappy changelogs, we
> would benefit from having a standard changelog template.
>
> Something which guides people to creating a good changelog. But it
> would have to be short, and carefully written. It should learn from
> history, to wit:
>
> - ./REPORTING-BUGS has a template and afaik it has never elicited any
> useful information.
>
> - Documentation/SubmittingPatches has info on how to write a
> changelog, and people blithely ignore it.
>
> - kerneldoc provide a template of sorts, and we see that filling out
> templates puts people's brains into "filling out a template" mode,
> rather than into "communicating information" mode.
>
> An interesting problem.
Well, if git commit could add that template in addition to the
Signed-off-by line, that could be a start, perhaps as a new option and
then it would get it from .git/changelog-template, that would be
provided by each project.
/me scratches head...
-t, --template <FILE>
use specified template file
Its there already, but then perhaps what is needed is a _default_
template.
- Arnaldo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
2008-12-16 1:10 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Alexey Dobriyan
2008-12-16 1:19 ` Andrew Morton
2008-12-16 6:38 ` Impact: David Miller
@ 2008-12-16 23:00 ` Ingo Molnar
2 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2008-12-16 23:00 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: David Miller, rostedt, linux-kernel, dada1, akpm, acme
* Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Mon, Dec 15, 2008 at 12:20:19AM -0800, David Miller wrote:
> > > update rwlock initialization for nat_table
> > >
> > > Impact: clean up
> > >
> > > The commit e099a173573ce1ba171092aee7bb3c72ea686e59
> > > (netfilter: netns nat: per-netns NAT table) renamed the
> > > nat_table from __nat_table to nat_table without updating the
> > > __RW_LOCK_UNLOCKED(__nat_table.lock).
> > >
> > > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> >
> > Applied to net-2.6, thanks Steven.
> >
> > As Andrew mentioned this is a bug (albeit a "nano-bug" as you
> > called it :-) so I removed the Impact line in the commit
> > message when applying this.
>
> Speaking of Impact: lines, is this a new fashion or what?
>
> Looking at the ones which are already in official tree, they are either
> trivially duplicating Subject: line, or effectively duplicating Subject:
> line, or cover up for insufficiently informative (read: badly written)
> Subject: line, or simply useless.
> Subject: sched: CPU remove deadlock fix
> Impact: fix possible deadlock in CPU hot-remove path
>
> What prevented to write "Subject: sched: fix possible deadlock in CPU
> hot-remove path"?
there are 655 Impact lines, right now, and we find them rather useful, for
a multitude of reasons. The impact line is a secondary subject line in
essence, describing the intended scope and practical impact of a change -
and not describe the change itself. The advantages are:
- they encourage a proper splitup of patches:
- the more complex a patch is, the harder it is to write a proper
impact line for it. So when people complain to me that they find it
hard to describe the practical impact of a patch in a single line, i
ask them to split up the patch ;-)
- they standardize and document impact analysis:
- it helps -stable later on in filtering out fixes and ordering
them by risk. We might not want to mark a commit as Cc: stable
straight away - but we want to describe the risk analysis we have
performed.
- they also help filtering out the patches that go into -git versus
devel stuff.
- they help bug analysis:
- Impact lines make it abundantly clear what the intented scope of a
change was, on the first line of the commit. We had incidents in the
past where people bisected to a commit and were wondering whether a
change was intended to have side-effects or not - and the Impact
line made it clear that the side-effect was not intended.
- they help bisection itself too: a couple of times i used it already
to home in on a suspected change that introduced a breakage. If
there's a material change in the middle of cleanups, it's hard to
see that in the changelogs immediately.
- they ease review:
- when a patch comes in that has an Impact line, i just look at the
impact line and match it up with what the patch actually does. If
there's mismatch it raises a red flag. Subject lines and changelogs
come from dozens and dozens of different authors, with different
cultural and language backgrounds, with different levels of
experience. It's much easier (and faster) to approach a patch with
an impact line from the right angle.
- they make it much harder to apply patches without a proper
level of review. Creating a good impact line is a good last line of
defense both at the submitter and at the applier level.
- they make it clear to the patch submitter if i mis-judge a change.
They tell me when i create the wrong Impact line and i can
re-consider the change.
That is an overlapping but still different purpose from a subject line.
Subject lines are controlled by the 'what' and 'how' questions of a code
change - while the Impact line is only controlled by the: 'risk'/'impact'
aspect.
Subject lines are also often controlled by subsystem maintenance
tradition, have various tags to distract from, etc. We try to match up
subject lines close to the lkml subject were they were discussed - and
only change them if they are really bad. That linkage is important.
Appending and prepending impact information gets messy.
All kernel developers and maintainers who started using them (whom i
talked to) found them rather useful - even if they had reservations about
the seemingly duplicated subject line aspect in the beginning. I dont
think you can judge this from an armchair - as you are only the reader of
an impact line, not the creator of it. At least half of the good stuff
happens while you active create them. Try it if you dont believe me ;-) In
any case, you dont have to use it if you dont like it.
Ingo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-12-16 23:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-10 18:21 [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c Steven Rostedt
2008-12-10 18:33 ` Eric Dumazet
2008-12-10 19:00 ` Steven Rostedt
2008-12-10 20:06 ` [PATCH] update rwlock initialization for nat_table Steven Rostedt
2008-12-11 22:18 ` Andrew Morton
2008-12-11 22:24 ` Steven Rostedt
2008-12-15 8:20 ` David Miller
2008-12-16 1:10 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Alexey Dobriyan
2008-12-16 1:19 ` Andrew Morton
2008-12-16 11:15 ` Arnaldo Carvalho de Melo
2008-12-16 6:38 ` Impact: David Miller
2008-12-16 23:00 ` Impact: (was Re: [PATCH] update rwlock initialization for nat_table) Ingo Molnar
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.