Return-Path: Delivered-To: kernel@kolivas.org Received: from bhhdoa.org.au (bhhdoa.org.au [216.17.101.199]) by mail.kolivas.org (Postfix) with ESMTP id A7F4A3E031 for ; Fri, 9 Jul 2004 17:51:00 +1000 (EST) Received: from virtualhost.dk (ns.virtualhost.dk [195.184.98.160]) by bhhdoa.org.au (Postfix) with ESMTP id 1B78F50B4C for ; Fri, 9 Jul 2004 15:24:47 +1000 (EST) Received: from [62.242.22.158] (helo=mcbain.home.kernel.dk) by virtualhost.dk with esmtp (Exim 3.36 #1) id 1Biq9n-0008MN-00 for kernel@kolivas.org; Fri, 09 Jul 2004 09:50:55 +0200 Received: from axboe by mcbain.home.kernel.dk with local (Exim 4.30) id 1Biq9T-0002iX-6a for kernel@kolivas.org; Fri, 09 Jul 2004 09:50:35 +0200 Date: Fri, 9 Jul 2004 09:50:35 +0200 From: Jens Axboe To: kernel@kolivas.org Subject: [axboe@suse.de: Re: cfq patch is broken] Message-ID: <20040709075034.GB10114@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Con, JFYI, there was a buglet in the version posted the other day. This is a replacement. ----- Forwarded message from Jens Axboe ----- From: Jens Axboe Date: Fri, 9 Jul 2004 09:06:41 +0200 To: Arjan van de Ven Cc: akpm@osdl.org Subject: Re: cfq patch is broken On Fri, Jul 09 2004, Jens Axboe wrote: > On Thu, Jul 08 2004, Arjan van de Ven wrote: > > Unable to handle kernel paging request at virtual address 6b6b6b6b > > printing eip: > > 022386a9 > > *pde = 00000000 > > Oops: 0000 [#1] > > Modules linked in: snd_mixer_oss ipv6 parport_pc lp parport autofs4 > > orinoco_cs orinoco hermes ds yenta_socket pcmcia_core sunrpc ipt_REJECT > > iptable_filter ip_tables floppy sg scsi_mod microcode snd_intel8x0 > > snd_ac97_codec snd_pcm snd_timer snd_page_alloc gameport joydev > > snd_mpu401_uart snd_rawmidi snd_seq_device snd soundcore uhci_hcd 3c59x > > eeprom i2c_sensor i2c_i801 i2c_core dm_mod button battery asus_acpi ac ext3 > > jbd > > CPU: 0 > > EIP: 0060:[<022386a9>] Not tainted > > EFLAGS: 00010212 (2.6.7-1.476) > > EIP is at cfq_get_queue+0x28/0x98 > > Locking is broken, hang on... This should work. Index: linux-2.6.7-ck/drivers/block/cfq-iosched.c =================================================================== --- linux-2.6.7-ck.orig/drivers/block/cfq-iosched.c 2004-07-30 10:47:17.142206787 +1000 +++ linux-2.6.7-ck/drivers/block/cfq-iosched.c 2004-07-30 10:47:18.121053387 +1000 @@ -456,37 +456,61 @@ mempool_free(cfqq, cfq_mpool); } -static struct cfq_queue *cfq_get_queue(struct cfq_data *cfqd, int pid) +static struct cfq_queue *__cfq_get_queue(struct cfq_data *cfqd, int pid, + int gfp_mask) { const int hashval = hash_long(current->tgid, CFQ_QHASH_SHIFT); struct cfq_queue *cfqq = __cfq_find_cfq_hash(cfqd, pid, hashval); if (!cfqq) { - cfqq = mempool_alloc(cfq_mpool, GFP_NOIO); + cfqq = mempool_alloc(cfq_mpool, gfp_mask); - INIT_LIST_HEAD(&cfqq->cfq_hash); - INIT_LIST_HEAD(&cfqq->cfq_list); - RB_CLEAR_ROOT(&cfqq->sort_list); - - cfqq->pid = pid; - cfqq->queued[0] = cfqq->queued[1] = 0; - list_add(&cfqq->cfq_hash, &cfqd->cfq_hash[hashval]); + if (cfqq) { + INIT_LIST_HEAD(&cfqq->cfq_hash); + INIT_LIST_HEAD(&cfqq->cfq_list); + RB_CLEAR_ROOT(&cfqq->sort_list); + + cfqq->pid = pid; + cfqq->queued[0] = cfqq->queued[1] = 0; + list_add(&cfqq->cfq_hash, &cfqd->cfq_hash[hashval]); + } } return cfqq; } -static void cfq_enqueue(struct cfq_data *cfqd, struct cfq_rq *crq) +static struct cfq_queue *cfq_get_queue(struct cfq_data *cfqd, int pid, + int gfp_mask) { + request_queue_t *q = cfqd->queue; struct cfq_queue *cfqq; - cfqq = cfq_get_queue(cfqd, current->tgid); + spin_lock_irq(q->queue_lock); + cfqq = __cfq_get_queue(cfqd, pid, gfp_mask); + spin_unlock_irq(q->queue_lock); + + return cfqq; +} + +static void cfq_enqueue(struct cfq_data *cfqd, struct cfq_rq *crq) +{ + struct cfq_queue *cfqq; - cfq_add_crq_rb(cfqd, cfqq, crq); + cfqq = __cfq_get_queue(cfqd, current->tgid, GFP_ATOMIC); + if (cfqq) { + cfq_add_crq_rb(cfqd, cfqq, crq); - if (list_empty(&cfqq->cfq_list)) { - list_add(&cfqq->cfq_list, &cfqd->rr_list); - cfqd->busy_queues++; + if (list_empty(&cfqq->cfq_list)) { + list_add(&cfqq->cfq_list, &cfqd->rr_list); + cfqd->busy_queues++; + } + } else { + /* + * should can only happen if the request wasn't allocated + * through blk_alloc_request(), eg stack requests from ide-cd + * (those should be removed) _and_ we are in OOM. + */ + list_add_tail(&crq->request->queuelist, cfqd->dispatch); } } @@ -617,8 +641,17 @@ static int cfq_set_request(request_queue_t *q, struct request *rq, int gfp_mask) { struct cfq_data *cfqd = q->elevator.elevator_data; - struct cfq_rq *crq = mempool_alloc(cfqd->crq_pool, gfp_mask); + struct cfq_queue *cfqq; + struct cfq_rq *crq; + + /* + * prepare a queue up front, so cfq_enqueue() doesn't have to + */ + cfqq = cfq_get_queue(cfqd, current->tgid, gfp_mask); + if (!cfqq) + return 1; + crq = mempool_alloc(cfqd->crq_pool, gfp_mask); if (crq) { RB_CLEAR(&crq->rb_node); crq->request = rq;