Re: Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
- From: Ingo Molnar <mingo@xxxxxxx>
- Date: Wed, 17 Dec 2008 00:00:09 +0100
* Alexey Dobriyan <adobriyan@xxxxxxxxx> 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@xxxxxxxxxx>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- References:
- [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
- From: Steven Rostedt
- Re: [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
- From: Eric Dumazet
- [PATCH] update rwlock initialization for nat_table
- From: Steven Rostedt
- Re: [PATCH] update rwlock initialization for nat_table
- From: David Miller
- Impact: (was Re: [PATCH] update rwlock initialization for nat_table)
- From: Alexey Dobriyan
- [PATCH] replace deprecated RW_LOCK_UNLOCKED in net/dccp/proto.c
- Prev by Date: Re: [PATCH] tracing/ftrace: use preempt_enable_no_resched_notrace in ring_buffer_time_stamp()
- Next by Date: Re: [patch] Performance Counters for Linux, v4
- Previous by thread: Re: Impact:
- Next by thread: [PATCH 00/15] Kernel memory leak detector
- Index(es):
Relevant Pages
|
