All of lore.kernel.org
 help / color / mirror / Atom feed
* [git pull] core/softirq for v2.6.27
@ 2008-07-14 14:42 Ingo Molnar
  2008-07-14 16:22 ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2008-07-14 14:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Thomas Gleixner, Andrew Morton

Linus,

Please pull the latest core/softirq changes git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/softirq

Thanks,

	Ingo

------------------>
Carlos R. Mafra (1):
      Remove argument from open_softirq which is always NULL

Johannes Berg (1):
      softirq: remove irqs_disabled warning from local_bh_enable

Vegard Nossum (1):
      softirq: remove initialization of static per-cpu variable


 block/blk-core.c          |    2 +-
 include/linux/interrupt.h |    3 +-
 kernel/hrtimer.c          |    2 +-
 kernel/rcuclassic.c       |    2 +-
 kernel/rcupreempt.c       |    2 +-
 kernel/sched.c            |    2 +-
 kernel/softirq.c          |   61 ++++++++++++---------------------------------
 kernel/timer.c            |    2 +-
 net/core/dev.c            |    4 +-
 9 files changed, 25 insertions(+), 55 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1905aab..af094ff 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2045,7 +2045,7 @@ int __init blk_dev_init(void)
 	for_each_possible_cpu(i)
 		INIT_LIST_HEAD(&per_cpu(blk_cpu_done, i));
 
-	open_softirq(BLOCK_SOFTIRQ, blk_done_softirq, NULL);
+	open_softirq(BLOCK_SOFTIRQ, blk_done_softirq);
 	register_hotcpu_notifier(&blk_cpu_notifier);
 
 	return 0;
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index f1fc747..a86186d 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -285,12 +285,11 @@ enum
 struct softirq_action
 {
 	void	(*action)(struct softirq_action *);
-	void	*data;
 };
 
 asmlinkage void do_softirq(void);
 asmlinkage void __do_softirq(void);
-extern void open_softirq(int nr, void (*action)(struct softirq_action*), void *data);
+extern void open_softirq(int nr, void (*action)(struct softirq_action *));
 extern void softirq_init(void);
 #define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
 extern void raise_softirq_irqoff(unsigned int nr);
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 421be5f..861b408 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1669,7 +1669,7 @@ void __init hrtimers_init(void)
 			  (void *)(long)smp_processor_id());
 	register_cpu_notifier(&hrtimers_nb);
 #ifdef CONFIG_HIGH_RES_TIMERS
-	open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq, NULL);
+	open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq);
 #endif
 }
 
diff --git a/kernel/rcuclassic.c b/kernel/rcuclassic.c
index f4ffbd0..f6e01f3 100644
--- a/kernel/rcuclassic.c
+++ b/kernel/rcuclassic.c
@@ -529,7 +529,7 @@ static void __cpuinit rcu_online_cpu(int cpu)
 
 	rcu_init_percpu_data(cpu, &rcu_ctrlblk, rdp);
 	rcu_init_percpu_data(cpu, &rcu_bh_ctrlblk, bh_rdp);
-	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks, NULL);
+	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
 }
 
 static int __cpuinit rcu_cpu_notify(struct notifier_block *self,
diff --git a/kernel/rcupreempt.c b/kernel/rcupreempt.c
index 5e02b77..902dce2 100644
--- a/kernel/rcupreempt.c
+++ b/kernel/rcupreempt.c
@@ -1123,7 +1123,7 @@ void __init __rcu_init(void)
 	for_each_online_cpu(cpu)
 		rcu_cpu_notify(&rcu_nb, CPU_UP_PREPARE,	(void *)(long) cpu);
 
-	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks, NULL);
+	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
 }
 
 /*
diff --git a/kernel/sched.c b/kernel/sched.c
index 3aaa5c8..5695835 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -7832,7 +7832,7 @@ void __init sched_init(void)
 #endif
 
 #ifdef CONFIG_SMP
-	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains, NULL);
+	open_softirq(SCHED_SOFTIRQ, run_rebalance_domains);
 #endif
 
 #ifdef CONFIG_RT_MUTEXES
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 36e0617..2cf2502 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -131,23 +131,17 @@ void _local_bh_enable(void)
 
 EXPORT_SYMBOL(_local_bh_enable);
 
-void local_bh_enable(void)
+static inline void _local_bh_enable_ip(unsigned long ip)
 {
+	WARN_ON_ONCE(in_irq() || irqs_disabled());
 #ifdef CONFIG_TRACE_IRQFLAGS
-	unsigned long flags;
-
-	WARN_ON_ONCE(in_irq());
-#endif
-	WARN_ON_ONCE(irqs_disabled());
-
-#ifdef CONFIG_TRACE_IRQFLAGS
-	local_irq_save(flags);
+	local_irq_disable();
 #endif
 	/*
 	 * Are softirqs going to be turned on now:
 	 */
 	if (softirq_count() == SOFTIRQ_OFFSET)
-		trace_softirqs_on((unsigned long)__builtin_return_address(0));
+		trace_softirqs_on(ip);
 	/*
 	 * Keep preemption disabled until we are done with
 	 * softirq processing:
@@ -159,40 +153,20 @@ void local_bh_enable(void)
 
 	dec_preempt_count();
 #ifdef CONFIG_TRACE_IRQFLAGS
-	local_irq_restore(flags);
+	local_irq_enable();
 #endif
 	preempt_check_resched();
 }
+
+void local_bh_enable(void)
+{
+	_local_bh_enable_ip((unsigned long)__builtin_return_address(0));
+}
 EXPORT_SYMBOL(local_bh_enable);
 
 void local_bh_enable_ip(unsigned long ip)
 {
-#ifdef CONFIG_TRACE_IRQFLAGS
-	unsigned long flags;
-
-	WARN_ON_ONCE(in_irq());
-
-	local_irq_save(flags);
-#endif
-	/*
-	 * Are softirqs going to be turned on now:
-	 */
-	if (softirq_count() == SOFTIRQ_OFFSET)
-		trace_softirqs_on(ip);
-	/*
-	 * Keep preemption disabled until we are done with
-	 * softirq processing:
- 	 */
- 	sub_preempt_count(SOFTIRQ_OFFSET - 1);
-
-	if (unlikely(!in_interrupt() && local_softirq_pending()))
-		do_softirq();
-
-	dec_preempt_count();
-#ifdef CONFIG_TRACE_IRQFLAGS
-	local_irq_restore(flags);
-#endif
-	preempt_check_resched();
+	_local_bh_enable_ip(ip);
 }
 EXPORT_SYMBOL(local_bh_enable_ip);
 
@@ -347,9 +321,8 @@ void raise_softirq(unsigned int nr)
 	local_irq_restore(flags);
 }
 
-void open_softirq(int nr, void (*action)(struct softirq_action*), void *data)
+void open_softirq(int nr, void (*action)(struct softirq_action *))
 {
-	softirq_vec[nr].data = data;
 	softirq_vec[nr].action = action;
 }
 
@@ -360,10 +333,8 @@ struct tasklet_head
 	struct tasklet_struct **tail;
 };
 
-/* Some compilers disobey section attribute on statics when not
-   initialized -- RR */
-static DEFINE_PER_CPU(struct tasklet_head, tasklet_vec) = { NULL };
-static DEFINE_PER_CPU(struct tasklet_head, tasklet_hi_vec) = { NULL };
+static DEFINE_PER_CPU(struct tasklet_head, tasklet_vec);
+static DEFINE_PER_CPU(struct tasklet_head, tasklet_hi_vec);
 
 void __tasklet_schedule(struct tasklet_struct *t)
 {
@@ -503,8 +474,8 @@ void __init softirq_init(void)
 			&per_cpu(tasklet_hi_vec, cpu).head;
 	}
 
-	open_softirq(TASKLET_SOFTIRQ, tasklet_action, NULL);
-	open_softirq(HI_SOFTIRQ, tasklet_hi_action, NULL);
+	open_softirq(TASKLET_SOFTIRQ, tasklet_action);
+	open_softirq(HI_SOFTIRQ, tasklet_hi_action);
 }
 
 static int ksoftirqd(void * __bind_cpu)
diff --git a/kernel/timer.c b/kernel/timer.c
index ceacc66..b4da888 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1502,7 +1502,7 @@ void __init init_timers(void)
 
 	BUG_ON(err == NOTIFY_BAD);
 	register_cpu_notifier(&timers_nb);
-	open_softirq(TIMER_SOFTIRQ, run_timer_softirq, NULL);
+	open_softirq(TIMER_SOFTIRQ, run_timer_softirq);
 }
 
 /**
diff --git a/net/core/dev.c b/net/core/dev.c
index c421a1f..48f1e04 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4585,8 +4585,8 @@ static int __init net_dev_init(void)
 
 	dev_boot_phase = 0;
 
-	open_softirq(NET_TX_SOFTIRQ, net_tx_action, NULL);
-	open_softirq(NET_RX_SOFTIRQ, net_rx_action, NULL);
+	open_softirq(NET_TX_SOFTIRQ, net_tx_action);
+	open_softirq(NET_RX_SOFTIRQ, net_rx_action);
 
 	hotcpu_notifier(dev_cpu_callback, 0);
 	dst_init();

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 14:42 [git pull] core/softirq for v2.6.27 Ingo Molnar
@ 2008-07-14 16:22 ` Andrew Morton
  2008-07-14 16:31   ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Morton @ 2008-07-14 16:22 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Linus Torvalds, linux-kernel, Thomas Gleixner

On Mon, 14 Jul 2008 16:42:43 +0200 Ingo Molnar <mingo@elte.hu> wrote:
>

It would be nice if these short-form summaries were to include the
commit IDs.  eg:

Carlos R. Mafra (1):
      Remove argument from open_softirq which is always NULL (962cf36)

> Carlos R. Mafra (1):
>       Remove argument from open_softirq which is always NULL
> 
> Johannes Berg (1):
>       softirq: remove irqs_disabled warning from local_bh_enable

This doesn't remove the warning?  (Nor should it)

+static inline void _local_bh_enable_ip(unsigned long ip)
 {
+       WARN_ON_ONCE(in_irq() || irqs_disabled());
-       WARN_ON_ONCE(in_irq());
-       WARN_ON_ONCE(irqs_disabled());

It just makes it a bit less useful.
	
> Vegard Nossum (1):
>       softirq: remove initialization of static per-cpu variable

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 16:22 ` Andrew Morton
@ 2008-07-14 16:31   ` Ingo Molnar
  2008-07-14 16:44     ` Andrew Morton
  0 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2008-07-14 16:31 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Linus Torvalds, linux-kernel, Thomas Gleixner


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 14 Jul 2008 16:42:43 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> >
> 
> It would be nice if these short-form summaries were to include the
> commit IDs.  eg:
> 
> Carlos R. Mafra (1):
>       Remove argument from open_softirq which is always NULL (962cf36)

yeah, but not sure whether git-shortlog can do that and this shortlog is 
the Preferred Way of getting pull requests.

I'm using the sha1's myself for non-shortlog formats, like:

  earth4:~/tip> git-log-line linus..core/softirq
  0f476b6: softirq: remove irqs_disabled warning from local_bh_enable
  4620b49: softirq: remove initialization of static per-cpu variable
  962cf36: Remove argument from open_softirq which is always NULL

where git-log-line does the obvious single-line shortcut:

  git log --no-merges --pretty=format:"%h: %s" $@

> > Carlos R. Mafra (1):
> >       Remove argument from open_softirq which is always NULL
> > 
> > Johannes Berg (1):
> >       softirq: remove irqs_disabled warning from local_bh_enable
> 
> This doesn't remove the warning?  (Nor should it)
> 
> +static inline void _local_bh_enable_ip(unsigned long ip)
>  {
> +       WARN_ON_ONCE(in_irq() || irqs_disabled());
> -       WARN_ON_ONCE(in_irq());
> -       WARN_ON_ONCE(irqs_disabled());
> 
> It just makes it a bit less useful.

hm, it does more than that - this commit (0f476b6d9) actually caught a 
real bug/lockup on one of my testboxes, see this (in v2.6.26) fix:

| commit c5643cab7bf663ae049b11be43de8819683176dd
| Author: Ingo Molnar <mingo@elte.hu>
| Date:   Mon Jun 23 10:41:23 2008 +0200
|
|    [netdrvr] 3c59x: remove irqs_disabled warning from local_bh_enable
|
|    Original Author: Michael Buesch <mb@bu3sch.de>
|
|    net, vortex: fix lockup

[ i didnt send it to v2.6.26 because it came relatively late, it touches 
  a sensitive, #ifdef laden piece of code and it was unknown how many 
  false positives it would trigger. But it's perfect for v2.6.27. ]

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 16:31   ` Ingo Molnar
@ 2008-07-14 16:44     ` Andrew Morton
  2008-07-14 16:48       ` Ingo Molnar
  2008-07-14 17:11       ` Linus Torvalds
  0 siblings, 2 replies; 12+ messages in thread
From: Andrew Morton @ 2008-07-14 16:44 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Linus Torvalds, linux-kernel, Thomas Gleixner

On Mon, 14 Jul 2008 18:31:41 +0200 Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Mon, 14 Jul 2008 16:42:43 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> > >
> > 
> > It would be nice if these short-form summaries were to include the
> > commit IDs.  eg:
> > 
> > Carlos R. Mafra (1):
> >       Remove argument from open_softirq which is always NULL (962cf36)
> 
> yeah, but not sure whether git-shortlog

This was a git-shortlog feature request ;)

> can do that and this shortlog is 
> the Preferred Way of getting pull requests.
> 
> I'm using the sha1's myself for non-shortlog formats, like:
> 
>   earth4:~/tip> git-log-line linus..core/softirq
>   0f476b6: softirq: remove irqs_disabled warning from local_bh_enable
>   4620b49: softirq: remove initialization of static per-cpu variable
>   962cf36: Remove argument from open_softirq which is always NULL
> 
> where git-log-line does the obvious single-line shortcut:
> 
>   git log --no-merges --pretty=format:"%h: %s" $@
> 
> > > Carlos R. Mafra (1):
> > >       Remove argument from open_softirq which is always NULL
> > > 
> > > Johannes Berg (1):
> > >       softirq: remove irqs_disabled warning from local_bh_enable
> > 
> > This doesn't remove the warning?  (Nor should it)
> > 
> > +static inline void _local_bh_enable_ip(unsigned long ip)
> >  {
> > +       WARN_ON_ONCE(in_irq() || irqs_disabled());
> > -       WARN_ON_ONCE(in_irq());
> > -       WARN_ON_ONCE(irqs_disabled());
> > 
> > It just makes it a bit less useful.
> 
> hm, it does more than that

Yes, but it does what I said:

- Doesn't do something which the changelog said it did.  I brought this up
  because if the patch _did_ do what it said it did, davem would cry.

- Makes the warning less useful.  The reason for the warning can still
  probably be worked out from the backtrace, but it'll take more work.

> - this commit (0f476b6d9) actually caught a 
> real bug/lockup on one of my testboxes, see this (in v2.6.26) fix:
> 
> | commit c5643cab7bf663ae049b11be43de8819683176dd
> | Author: Ingo Molnar <mingo@elte.hu>
> | Date:   Mon Jun 23 10:41:23 2008 +0200
> |
> |    [netdrvr] 3c59x: remove irqs_disabled warning from local_bh_enable

OK, that title kinda makes sense.

> |    Original Author: Michael Buesch <mb@bu3sch.de>
> |
> |    net, vortex: fix lockup

Although that would be a better title for the 3c59x.c fix.

> [ i didnt send it to v2.6.26 because it came relatively late, it touches 
>   a sensitive, #ifdef laden piece of code and it was unknown how many 
>   false positives it would trigger. But it's perfect for v2.6.27. ]
> 

It's a small issue - it's just that the title of
0f476b6d91a1395bda6464e653ce66ea9bea7167 ("softirq: remove
irqs_disabled warning from local_bh_enable") appears to be quite
wrong.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 16:44     ` Andrew Morton
@ 2008-07-14 16:48       ` Ingo Molnar
  2008-07-14 17:13         ` Linus Torvalds
  2008-07-14 17:11       ` Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2008-07-14 16:48 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Linus Torvalds, linux-kernel, Thomas Gleixner


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 14 Jul 2008 18:31:41 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Mon, 14 Jul 2008 16:42:43 +0200 Ingo Molnar <mingo@elte.hu> wrote:
> > > >
> > > 
> > > It would be nice if these short-form summaries were to include the
> > > commit IDs.  eg:
> > > 
> > > Carlos R. Mafra (1):
> > >       Remove argument from open_softirq which is always NULL (962cf36)
> > 
> > yeah, but not sure whether git-shortlog
> 
> This was a git-shortlog feature request ;)
> 
> > can do that and this shortlog is 
> > the Preferred Way of getting pull requests.
> > 
> > I'm using the sha1's myself for non-shortlog formats, like:
> > 
> >   earth4:~/tip> git-log-line linus..core/softirq
> >   0f476b6: softirq: remove irqs_disabled warning from local_bh_enable
> >   4620b49: softirq: remove initialization of static per-cpu variable
> >   962cf36: Remove argument from open_softirq which is always NULL
> > 
> > where git-log-line does the obvious single-line shortcut:
> > 
> >   git log --no-merges --pretty=format:"%h: %s" $@
> > 
> > > > Carlos R. Mafra (1):
> > > >       Remove argument from open_softirq which is always NULL
> > > > 
> > > > Johannes Berg (1):
> > > >       softirq: remove irqs_disabled warning from local_bh_enable
> > > 
> > > This doesn't remove the warning?  (Nor should it)
> > > 
> > > +static inline void _local_bh_enable_ip(unsigned long ip)
> > >  {
> > > +       WARN_ON_ONCE(in_irq() || irqs_disabled());
> > > -       WARN_ON_ONCE(in_irq());
> > > -       WARN_ON_ONCE(irqs_disabled());
> > > 
> > > It just makes it a bit less useful.
> > 
> > hm, it does more than that
> 
> Yes, but it does what I said:
> 
> - Doesn't do something which the changelog said it did.  I brought this up
>   because if the patch _did_ do what it said it did, davem would cry.
> 
> - Makes the warning less useful.  The reason for the warning can still
>   probably be worked out from the backtrace, but it'll take more work.
> 
> > - this commit (0f476b6d9) actually caught a 
> > real bug/lockup on one of my testboxes, see this (in v2.6.26) fix:
> > 
> > | commit c5643cab7bf663ae049b11be43de8819683176dd
> > | Author: Ingo Molnar <mingo@elte.hu>
> > | Date:   Mon Jun 23 10:41:23 2008 +0200
> > |
> > |    [netdrvr] 3c59x: remove irqs_disabled warning from local_bh_enable
> 
> OK, that title kinda makes sense.
> 
> > |    Original Author: Michael Buesch <mb@bu3sch.de>
> > |
> > |    net, vortex: fix lockup
> 
> Although that would be a better title for the 3c59x.c fix.
> 
> > [ i didnt send it to v2.6.26 because it came relatively late, it touches 
> >   a sensitive, #ifdef laden piece of code and it was unknown how many 
> >   false positives it would trigger. But it's perfect for v2.6.27. ]
> > 
> 
> It's a small issue - it's just that the title of 
> 0f476b6d91a1395bda6464e653ce66ea9bea7167 ("softirq: remove 
> irqs_disabled warning from local_bh_enable") appears to be quite 
> wrong.

ok, agreed.

/me suggests git-log-addendum feature to amend commit logs after the
    fact, without changing any of the code :-)

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 16:44     ` Andrew Morton
  2008-07-14 16:48       ` Ingo Molnar
@ 2008-07-14 17:11       ` Linus Torvalds
  2008-07-14 17:23         ` Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2008-07-14 17:11 UTC (permalink / raw)
  To: Andrew Morton, Junio C Hamano
  Cc: Ingo Molnar, Git Mailing List, Thomas Gleixner



On Mon, 14 Jul 2008, Andrew Morton wrote:
> 
> This was a git-shortlog feature request ;)

Heh. You should cc git or Junio if so.

The way shortlog is done (very different from regular logs, due to the 
whole organize-by-name etc thing) that is sadly pretty ugly to do: we 
don't support the pretty-formats etc. And the way things are done, adding 
support for the generic "--pretty=xyz" (including custom formats) would be 
pretty painful.

So it would have to be a total special-case.

Junio, git people - what Andrew is asking for is for git shortlog to show 
the commit ID (in shortened form) at the end:

> > It would be nice if these short-form summaries were to include the
> > commit IDs.  eg:
> > 
> > Carlos R. Mafra (1):
> >       Remove argument from open_softirq which is always NULL (962cf36)

and I do think it would be nice. I've had some other things I would find 
shortlog format additions useful for (size of patch etc), so if it could 
be somehow generalized that would be wonderful.

As it is, that is one ugly function that only takes the one-liner thing - 
at least partly because of how it traditionally worked (as a a filter over 
the log messages, rather than as a "git shortlog xyz..abc" kind of 
stand-alone thing).

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 16:48       ` Ingo Molnar
@ 2008-07-14 17:13         ` Linus Torvalds
  2008-07-14 18:23           ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Torvalds @ 2008-07-14 17:13 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Andrew Morton, linux-kernel, Thomas Gleixner



On Mon, 14 Jul 2008, Ingo Molnar wrote:
> 
> /me suggests git-log-addendum feature to amend commit logs after the
>     fact, without changing any of the code :-)

It's called "git commit --amend".

But it obviously also changes the SHA1 of the commit - it generates a 
totally new one. You cannot do it after-the-fact or deep in history (well, 
you can, with the normal "rebase" op, but it literally is no different 
from changing the patch itself too).

BK used to be able to change the logs after-the-fact, but that meant that 
the logs were not trustworthy, and any changes also didn't distribute 
right (they'd only be changed locally and in any subsequent distributions)

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 17:11       ` Linus Torvalds
@ 2008-07-14 17:23         ` Linus Torvalds
  2008-07-14 17:46           ` Johannes Schindelin
  2008-07-14 17:46           ` [git pull] core/softirq for v2.6.27 Linus Torvalds
  0 siblings, 2 replies; 12+ messages in thread
From: Linus Torvalds @ 2008-07-14 17:23 UTC (permalink / raw)
  To: Andrew Morton, Junio C Hamano
  Cc: Ingo Molnar, Git Mailing List, Thomas Gleixner



On Mon, 14 Jul 2008, Linus Torvalds wrote:
> 
> As it is, that is one ugly function that only takes the one-liner thing - 
> at least partly because of how it traditionally worked (as a a filter over 
> the log messages, rather than as a "git shortlog xyz..abc" kind of 
> stand-alone thing).

Heh. I guess you _could_ use the filter capability here (but it will 
sometimes truncate the line, so..)

Some really ugly scripting like this will do it:

	#!/bin/sh
	git log --pretty='format:commit %H
	Author: %an <%ae>
	
	  %s (%h)
	' $@ | git shortlog

where we use the regular "git log" with a user-supplied format to generate 
a log with the subject line and the shortened hash appended (the "%s (%h)" 
part) and then we use "git shortlog" as a filter to sort things together 
by author etc.

There is absolutely _nothing_ that git won't do with a little bit of 
scripting, but I do have to admit that this one isn't a wonderfully 
beautiful script ;)

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 17:23         ` Linus Torvalds
@ 2008-07-14 17:46           ` Johannes Schindelin
  2008-07-14 18:08             ` [PATCH] shortlog: support --pretty=format: option Johannes Schindelin
  2008-07-14 17:46           ` [git pull] core/softirq for v2.6.27 Linus Torvalds
  1 sibling, 1 reply; 12+ messages in thread
From: Johannes Schindelin @ 2008-07-14 17:46 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, Junio C Hamano, Ingo Molnar, Git Mailing List,
	Thomas Gleixner

Hi,

On Mon, 14 Jul 2008, Linus Torvalds wrote:

> 	git log --pretty='format:commit %H
> 	Author: %an <%ae>
> 	
> 	  %s (%h)
> 	' $@ | git shortlog

Surely you meant

 	git log --pretty='format:commit %H%nAuthor: %an <%ae>%n%n    %s (%h)' \
		$@ | git shortlog

Note that I do not think it would be all that hard to teach shortlog the 
--pretty option.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 17:23         ` Linus Torvalds
  2008-07-14 17:46           ` Johannes Schindelin
@ 2008-07-14 17:46           ` Linus Torvalds
  1 sibling, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2008-07-14 17:46 UTC (permalink / raw)
  To: Andrew Morton, Junio C Hamano
  Cc: Ingo Molnar, Git Mailing List, Thomas Gleixner



On Mon, 14 Jul 2008, Linus Torvalds wrote:
> 
> Some really ugly scripting like this will do it:
> 
> 	#!/bin/sh
> 	git log --pretty='format:commit %H
> 	Author: %an <%ae>

Side note: you can make it a one-liner with '%n' (not \n') if you want, ie

	#!/bin/sh
	git log --pretty='format:commit %H%nAuthor: %an <%ae>%n%n %s (%h)%n' $@ | git shortlog

if you want to make it look even more like line noise.

And if you want to be _really_ fancy, you can even make it a git 
alias. Just add somethign like this to your ~/.gitconfig file:

	[alias]
		mylog=!GIT_PAGER='git --no-pager shortlog' git log --pretty='format:commit %H%nAuthor: %an <%ae>%n%n %s (%h)%n'

which is the _real_ git-mans way to do it.

It does the "shortlog" output by using "git shortlog" as a pager, but then 
to avoid recursion, we have to turn off paging of shortlog output itself. 
Heh. And the reason to do that is that the arguments always go to the end, 
so since we want to give the args to "git log", we can't put the pipe at 
the end.

I will have to go wash up after writing the above.

		Linus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] shortlog: support --pretty=format: option
  2008-07-14 17:46           ` Johannes Schindelin
@ 2008-07-14 18:08             ` Johannes Schindelin
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2008-07-14 18:08 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, Junio C Hamano, Ingo Molnar, Git Mailing List,
	Thomas Gleixner


With this patch, the user can override the default setting, to print
the commit messages using a user format instead of the onelines of the
commits.  Example:

	$ git shortlog --pretty='format:%s (%h)' <commit>..

Note that shortlog will only respect a user format setting, as the other
formats do not make much sense.

Wished for by Andrew Morton.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 14 Jul 2008, Johannes Schindelin wrote:

	> Note that I do not think it would be all that hard to teach 
	> shortlog the --pretty option.

	Well...

 builtin-shortlog.c |   11 +++++++++++
 shortlog.h         |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 0136202..f8bcbfc 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -154,6 +154,15 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 	if (!author)
 		die("Missing author: %s",
 		    sha1_to_hex(commit->object.sha1));
+	if (log->user_format) {
+		struct strbuf buf = STRBUF_INIT;
+
+		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &buf,
+			DEFAULT_ABBREV, "", "", DATE_NORMAL, 0);
+		insert_one_record(log, author, buf.buf);
+		strbuf_release(&buf);
+		return;
+	}
 	if (*buffer)
 		buffer++;
 	insert_one_record(log, author, !*buffer ? "<none>" : buffer);
@@ -271,6 +280,8 @@ parse_done:
 		usage_with_options(shortlog_usage, options);
 	}
 
+	log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT;
+
 	/* assume HEAD if from a tty */
 	if (!nongit && !rev.pending.nr && isatty(0))
 		add_head_to_pending(&rev);
diff --git a/shortlog.h b/shortlog.h
index 31ff491..6608ee8 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -11,6 +11,7 @@ struct shortlog {
 	int wrap;
 	int in1;
 	int in2;
+	int user_format;
 
 	char *common_repo_prefix;
 	int email;
-- 
1.5.6.2.511.ge432a

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [git pull] core/softirq for v2.6.27
  2008-07-14 17:13         ` Linus Torvalds
@ 2008-07-14 18:23           ` Ingo Molnar
  0 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2008-07-14 18:23 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Andrew Morton, linux-kernel, Thomas Gleixner


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, 14 Jul 2008, Ingo Molnar wrote:
> > 
> > /me suggests git-log-addendum feature to amend commit logs after the
> >     fact, without changing any of the code :-)
> 
> It's called "git commit --amend".

yeah - and i use that for general development meanwhile: i just 
commit the draft code in small steps (appending it to the commit via
git commit --amend), then i test it and amend it with a final commit
log in the end.

I use that method instead of doing development via uncommitted changes 
in the git index, because that way:

 - I see the under-development change in a form close to how other
   people see it (as a commit)

 - I can switch branches during development

 - It's sure that changes are not lost

 - I can distribute my half-done commits to my testboxes (internally i
   use a hierarchy of Git repos on testboxes to automatically propagate
   changes)

Once it's pushed out publicly into a persistent branch it cannot be 
touched anymore.

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2008-07-14 18:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-14 14:42 [git pull] core/softirq for v2.6.27 Ingo Molnar
2008-07-14 16:22 ` Andrew Morton
2008-07-14 16:31   ` Ingo Molnar
2008-07-14 16:44     ` Andrew Morton
2008-07-14 16:48       ` Ingo Molnar
2008-07-14 17:13         ` Linus Torvalds
2008-07-14 18:23           ` Ingo Molnar
2008-07-14 17:11       ` Linus Torvalds
2008-07-14 17:23         ` Linus Torvalds
2008-07-14 17:46           ` Johannes Schindelin
2008-07-14 18:08             ` [PATCH] shortlog: support --pretty=format: option Johannes Schindelin
2008-07-14 17:46           ` [git pull] core/softirq for v2.6.27 Linus Torvalds

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.