Re: softirq parameters

george anzinger (george@mvista.com)
Sun, 04 Aug 2002 10:38:23 -0700


Matthew Wilcox wrote:
>
> what do you guys think about this patch? nobody's using the data argument
> to the softirq routines, but most of the routines want to know which
> CPU they're running on.

I would vote no on this. While no one is currently using
the data argument, it would be _hard_ to replace it if it
were needed. The cpu, on the other hand, is available
regardless of it being passed or not and thus does not
_need_ to be passed.

-g
>
> also, we have:
>
> static struct softirq_action softirq_vec[32] __cacheline_aligned_in_smp;
>
> and i rather wonder why we cache-align them. they only get initialised at
> startup (kernel or module init), so this cacheline must be in shared state.
>
> diff -urpNX dontdiff linux-2.5.30/drivers/scsi/scsi.c linux-2.5.30-willy/drivers/scsi/scsi.c
> --- linux-2.5.30/drivers/scsi/scsi.c 2002-07-06 07:21:00.000000000 -0600
> +++ linux-2.5.30-willy/drivers/scsi/scsi.c 2002-08-04 08:26:13.000000000 -0600
> @@ -1192,9 +1192,8 @@ void scsi_done(Scsi_Cmnd * SCpnt)
> * interrupt latency, stack depth, and reentrancy of the low-level
> * drivers.
> */
> -static void scsi_softirq(struct softirq_action *h)
> +static void scsi_softirq(int cpu)
> {
> - int cpu = smp_processor_id();
> struct softscsi_data *queue = &softscsi_data[cpu];
>
> while (queue->head) {
> @@ -2567,7 +2566,7 @@ static int __init init_scsi(void)
> bus_register(&scsi_driverfs_bus_type);
>
> /* Where we handle work queued by scsi_done */
> - open_softirq(SCSI_SOFTIRQ, scsi_softirq, NULL);
> + open_softirq(SCSI_SOFTIRQ, scsi_softirq);
>
> return 0;
> }
> diff -urpNX dontdiff linux-2.5.30/include/linux/interrupt.h linux-2.5.30-willy/include/linux/interrupt.h
> --- linux-2.5.30/include/linux/interrupt.h 2002-07-27 12:09:21.000000000 -0600
> +++ linux-2.5.30-willy/include/linux/interrupt.h 2002-07-27 12:12:52.000000000 -0600
> @@ -77,12 +77,11 @@ enum
>
> struct softirq_action
> {
> - void (*action)(struct softirq_action *);
> - void *data;
> + void (*action)(int cpu);
> };
>
> 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)(int cpu));
> extern void softirq_init(void);
> #define __cpu_raise_softirq(cpu, nr) do { softirq_pending(cpu) |= 1UL << (nr); } while (0)
> extern void FASTCALL(cpu_raise_softirq(unsigned int cpu, unsigned int nr));
> diff -urpNX dontdiff linux-2.5.30/kernel/softirq.c linux-2.5.30-willy/kernel/softirq.c
> --- linux-2.5.30/kernel/softirq.c 2002-08-02 05:44:53.000000000 -0600
> +++ linux-2.5.30-willy/kernel/softirq.c 2002-08-02 05:45:35.000000000 -0600
> @@ -86,7 +86,7 @@ restart:
>
> do {
> if (pending & 1)
> - h->action(h);
> + h->action(cpu);
> h++;
> pending >>= 1;
> } while (pending);
> @@ -136,9 +136,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)(int cpu))
> {
> - softirq_vec[nr].data = data;
> softirq_vec[nr].action = action;
> }
>
> @@ -176,7 +175,7 @@ void __tasklet_hi_schedule(struct taskle
> local_irq_restore(flags);
> }
>
> -static void tasklet_action(struct softirq_action *a)
> +static void tasklet_action(int cpu)
> {
> struct tasklet_struct *list;
>
> @@ -209,7 +208,7 @@ static void tasklet_action(struct softir
> }
> }
>
> -static void tasklet_hi_action(struct softirq_action *a)
> +static void tasklet_hi_action(int cpu)
> {
> struct tasklet_struct *list;
>
> @@ -321,8 +320,8 @@ void __init softirq_init()
> for (i=0; i<32; i++)
> tasklet_init(bh_task_vec+i, bh_action, i);
>
> - 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);
> }
>
> void __run_task_queue(task_queue *list)
> diff -urpNX dontdiff linux-2.5.30/net/core/dev.c linux-2.5.30-willy/net/core/dev.c
> --- linux-2.5.30/net/core/dev.c 2002-06-20 16:53:53.000000000 -0600
> +++ linux-2.5.30-willy/net/core/dev.c 2002-06-29 09:10:38.000000000 -0600
> @@ -1333,10 +1333,8 @@ static __inline__ void skb_bond(struct s
> skb->dev = dev->master;
> }
>
> -static void net_tx_action(struct softirq_action *h)
> +static void net_tx_action(int cpu)
> {
> - int cpu = smp_processor_id();
> -
> if (softnet_data[cpu].completion_queue) {
> struct sk_buff *clist;
>
> @@ -1573,9 +1571,8 @@ job_done:
> return 0;
> }
>
> -static void net_rx_action(struct softirq_action *h)
> +static void net_rx_action(int this_cpu)
> {
> - int this_cpu = smp_processor_id();
> struct softnet_data *queue = &softnet_data[this_cpu];
> unsigned long start_time = jiffies;
> int budget = netdev_max_backlog;
> @@ -2816,8 +2813,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);
>
> dst_init();
> dev_mcast_init();
>
> --
> Revolutions do not require corporate support.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

-- 
George Anzinger   george@mvista.com
High-res-timers: 
http://sourceforge.net/projects/high-res-timers/
Real time sched:  http://sourceforge.net/projects/rtsched/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/