Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932314Ab2FTSxR (ORCPT ); Wed, 20 Jun 2012 14:53:17 -0400 Received: from relay02ant.iops.be ([212.53.4.35]:47656 "EHLO relay02ant.iops.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751533Ab2FTSxP (ORCPT ); Wed, 20 Jun 2012 14:53:15 -0400 Message-ID: <4FE21C15.6040409@acm.org> Date: Wed, 20 Jun 2012 18:53:09 +0000 From: Bart Van Assche User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 MIME-Version: 1.0 To: Tejun Heo CC: Muthu Kumar , Jens Axboe , Jej B , linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org Subject: Re: [PATCH UPDATED] block: In blk_execute_rq_nowait, init rq->end_io before checking for dead queue. References: <4FD345DC.6020405@acm.org> <4FD4DC26.5080902@acm.org> <20120618224253.GH32733@google.com> In-Reply-To: <20120618224253.GH32733@google.com> X-Enigmail-Version: 1.5pre Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3174 Lines: 92 On 06/18/12 22:42, Tejun Heo wrote: > On Mon, Jun 11, 2012 at 02:23:23PM -0700, Muthu Kumar wrote: >> On Mon, Jun 11, 2012 at 10:33 AM, Muthu Kumar wrote: >>> On Sun, Jun 10, 2012 at 10:40 AM, Bart Van Assche wrote: >>>> On 06/09/12 23:57, Muthu Kumar wrote: >>>> >>>>> Locking change is the one you posted already (the link above). Anyway, >>>>> I have the attached patch *including* the locking change. Original >>>>> mail has attachment without locking change. Please use whatever you >>>>> need. >>>> >>>> While we are at it ... the rq->rq_disk and rq->end_io assignments can be >>>> performed safely before the spinlock is taken, isn't it ? >>> >>> Yes.. that spinlock is to protect the q. >> >> Attached patch with assignment performed before taking the spinlock. > > This looks correct to me. Bart, can you please include this patch in > your series and repost the series? I'll start testing the patch below: [PATCH] block: Fix blk_execute_rq_nowait() dead queue handling Make sure that blk_execute_rq_nowait() invokes the proper end_io function if the queue is dead and also that this call is performed with the queue lock held. Found this through source code review. Notes: - Since blk_execute_rq_nowait() must not be invoked from interrupt context it is safe to invoke end_io directly from this function. - blk_finish_request() already invokes request.end_io with the queue lock held. Signed-off-by: Bart Van Assche Reported-by: Muthukumar Ratty Cc: Jens Axboe Cc: Tejun Heo --- block/blk-exec.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/block/blk-exec.c b/block/blk-exec.c index fb2cbd5..c0fd83a 100644 --- a/block/blk-exec.c +++ b/block/blk-exec.c @@ -42,7 +42,8 @@ static void blk_end_sync_rq(struct request *rq, int error) * * Description: * Insert a fully prepared request at the back of the I/O scheduler queue - * for execution. Don't wait for completion. + * for execution. Don't wait for completion. This function may invoke + * rq->end_io directly. */ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, struct request *rq, int at_head, @@ -51,18 +52,20 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK; WARN_ON(irqs_disabled()); + + rq->rq_disk = bd_disk; + rq->end_io = done; + spin_lock_irq(q->queue_lock); if (unlikely(blk_queue_dead(q))) { - spin_unlock_irq(q->queue_lock); rq->errors = -ENXIO; if (rq->end_io) rq->end_io(rq, rq->errors); + spin_unlock_irq(q->queue_lock); return; } - rq->rq_disk = bd_disk; - rq->end_io = done; __elv_add_request(q, rq, where); __blk_run_queue(q); /* the queue is stopped so it won't be run */ -- 1.7.7 -- 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/