Re: [patch] smptimers, old BH removal, tq-cleanup, 2.5.39

Christoph Hellwig
2002-09-30T16:18:52Z

On Sun, Sep 29, 2002 at 07:52:17PM +0200, Ingo Molnar wrote:
> i've done the following cleanups/simplifications to task-queues:
>
> - removed the ability to define your own task-queue, what can be done is
> to schedule_task() a given task to keventd, and to flush all pending
> tasks.
>
> this is actually a quite easy transition, since 90% of all task-queue
> users in the kernel used BH_IMMEDIATE - which is very similar in
> functionality to keventd.

But it breaks XFS. Chris Wedgwood fixed it to use schedule_task()
instead (and I cleaned it up a littler more, see patch below), but
this does effectively simgle-thead XFS I/O completion.

Why? XFS used to have a per-cpu task-queue, emptied by a per-cpu
kernel thread. To get anywhere near the per-formance again we'd
need to merge the per-cpu keventd patch Arjan did for RH 2.1AS into
2.5. Altennatively we could allow kernel code to create it's own
kevends with associated task-queues, but that sounds rather ugly..

diff -uNr -p linux-2.5.39-mm1/fs/xfs/pagebuf/page_buf.c linux-2.5.39/fs/xfs/pagebuf/page_buf.c
--- linux-2.5.39-mm1/fs/xfs/pagebuf/page_buf.c Mon Sep 30 14:27:25 2002
+++ linux-2.5.39/fs/xfs/pagebuf/page_buf.c Mon Sep 30 16:49:06 2002
@@ -160,8 +160,6 @@ pb_tracking_free(

STATIC kmem_cache_t *pagebuf_cache;
STATIC pagebuf_daemon_t *pb_daemon;
-STATIC struct list_head pagebuf_iodone_tq[NR_CPUS];
-STATIC wait_queue_head_t pagebuf_iodone_wait[NR_CPUS];
STATIC void pagebuf_daemon_wakeup(int);

/*
@@ -1157,15 +1155,6 @@ _pagebuf_wait_unpin(
current->state = TASK_RUNNING;
}

-void
-pagebuf_queue_task(
- struct tq_struct *task)
-{
- queue_task(task, &pagebuf_iodone_tq[smp_processor_id()]);
- wake_up(&pagebuf_iodone_wait[smp_processor_id()]);
-}
-
-
/*
* Buffer Utility Routines
*/
@@ -1210,9 +1199,8 @@ pagebuf_iodone(
INIT_TQUEUE(&pb->pb_iodone_sched,
pagebuf_iodone_sched, (void *)pb);

- queue_task(&pb->pb_iodone_sched,
- &pagebuf_iodone_tq[smp_processor_id()]);
- wake_up(&pagebuf_iodone_wait[smp_processor_id()]);
+ schedule_task(&pb->pb_iodone_sched);
+
} else {
up(&pb->pb_iodonesema);
}
@@ -1666,62 +1654,6 @@ pagebuf_delwri_dequeue(
spin_unlock(&pb_daemon->pb_delwrite_lock);
}

-
-/*
- * The pagebuf iodone daemon
- */
-
-STATIC int pb_daemons[NR_CPUS];
-
-STATIC int
-pagebuf_iodone_daemon(
- void *__bind_cpu)
-{
- int cpu = (long) __bind_cpu;
- DECLARE_WAITQUEUE (wait, current);
-
- /* Set up the thread */
- daemonize();
-
- /* Avoid signals */
- spin_lock_irq(&current->sig->siglock);
- sigfillset(&current->blocked);
- recalc_sigpending();
- spin_unlock_irq(&current->sig->siglock);
-
- /* Migrate to the right CPU */
- set_cpus_allowed(current, 1UL << cpu);
- if (smp_processor_id() != cpu)
- BUG();
-
- sprintf(current->comm, "pagebuf_io_CPU%d", cpu);
- INIT_LIST_HEAD(&pagebuf_iodone_tq[cpu]);
- init_waitqueue_head(&pagebuf_iodone_wait[cpu]);
- __set_current_state(TASK_INTERRUPTIBLE);
- mb();
-
- pb_daemons[cpu] = 1;
-
- for (;;) {
- add_wait_queue(&pagebuf_iodone_wait[cpu],
- &wait);
-
- if (TQ_ACTIVE(pagebuf_iodone_tq[cpu]))
- __set_task_state(current, TASK_RUNNING);
- schedule();
- remove_wait_queue(&pagebuf_iodone_wait[cpu],
- &wait);
- run_task_queue(&pagebuf_iodone_tq[cpu]);
- if (pb_daemons[cpu] == 0)
- break;
- __set_current_state(TASK_INTERRUPTIBLE);
- }
-
- pb_daemons[cpu] = -1;
- wake_up_interruptible(&pagebuf_iodone_wait[cpu]);
- return 0;
-}
-
/* Defines for pagebuf daemon */
DECLARE_WAIT_QUEUE_HEAD(pbd_waitq);
STATIC int force_flush;
@@ -1907,8 +1839,6 @@ STATIC int
pagebuf_daemon_start(void)
{
if (!pb_daemon) {
- int cpu;
-
pb_daemon = (pagebuf_daemon_t *)
kmalloc(sizeof(pagebuf_daemon_t), GFP_KERNEL);
if (!pb_daemon) {
@@ -1924,19 +1854,6 @@ pagebuf_daemon_start(void)

kernel_thread(pagebuf_daemon, (void *)pb_daemon,
CLONE_FS|CLONE_FILES|CLONE_VM);
- for (cpu = 0; cpu < NR_CPUS; cpu++) {
- if (!cpu_online(cpu))
- continue;
- if (kernel_thread(pagebuf_iodone_daemon,
- (void *)(long) cpu,
- CLONE_FS|CLONE_FILES|CLONE_VM) < 0) {
- printk("pagebuf_daemon_start failed\n");
- } else {
- while (!pb_daemons[cpu]) {
- yield();
- }
- }
- }
}
return 0;
}
@@ -1950,24 +1867,12 @@ STATIC void
pagebuf_daemon_stop(void)
{
if (pb_daemon) {
- int cpu;
-
pb_daemon->active = 0;
pb_daemon->io_active = 0;

wake_up_interruptible(&pbd_waitq);
while (pb_daemon->active == 0) {
interruptible_sleep_on(&pbd_waitq);
- }
- for (cpu = 0; cpu < NR_CPUS; cpu++) {
- if (!cpu_online(cpu))
- continue;
- pb_daemons[cpu] = 0;
- wake_up(&pagebuf_iodone_wait[cpu]);
- while (pb_daemons[cpu] != -1) {
- interruptible_sleep_on(
- &pagebuf_iodone_wait[cpu]);
- }
}

kfree(pb_daemon);
diff -uNr -p linux-2.5.39-mm1/fs/xfs/pagebuf/page_buf.h linux-2.5.39/fs/xfs/pagebuf/page_buf.h
--- linux-2.5.39-mm1/fs/xfs/pagebuf/page_buf.h Mon Sep 30 14:13:46 2002
+++ linux-2.5.39/fs/xfs/pagebuf/page_buf.h Mon Sep 30 16:15:25 2002
@@ -324,9 +324,6 @@ extern void pagebuf_unlock( /* unlock b

#define pagebuf_geterror(pb) ((pb)->pb_error)

-extern void pagebuf_queue_task(
- struct tq_struct *);
-
extern void pagebuf_iodone( /* mark buffer I/O complete */
page_buf_t *); /* buffer to mark */

diff -uNr -p linux-2.5.39-mm1/fs/xfs/xfs_log.c linux-2.5.39/fs/xfs/xfs_log.c
--- linux-2.5.39-mm1/fs/xfs/xfs_log.c Mon Sep 30 14:15:55 2002
+++ linux-2.5.39/fs/xfs/xfs_log.c Mon Sep 30 16:15:25 2002
@@ -2779,7 +2779,7 @@ xlog_state_release_iclog(xlog_t *log,
case 0:
return xlog_sync(log, iclog, 0);
case 1:
- pagebuf_queue_task(&iclog->ic_write_sched);
+ schedule_task(&iclog->ic_write_sched);
}
}
return (0);