* [PATCH] ide: no need to check for the rq pointer to be NULL
@ 2011-01-09 23:51 Vasily Tarasov
2011-01-10 12:25 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Vasily Tarasov @ 2011-01-09 23:51 UTC (permalink / raw)
To: davem; +Cc: linux-ide
From: Vasily Tarasov <sectorsize512@gmail.com>
No need to check for the rq pointer to be NULL before calling
blk_fetch_request(). First we call it, then we test the pointer that it
returns.
Signed-off-by: Vasily Tarasov <sectorsize512@gmail.com>
---
--- linux-2.6.37/drivers/ide/ide-io.c.orig 2011-01-04 19:50:19.000000000 -0500
+++ linux-2.6.37/drivers/ide/ide-io.c 2011-01-09 18:15:44.000000000 -0500
@@ -438,7 +438,7 @@ void do_ide_request(struct request_queue
ide_drive_t *drive = q->queuedata;
ide_hwif_t *hwif = drive->hwif;
struct ide_host *host = hwif->host;
- struct request *rq = NULL;
+ struct request *rq;
ide_startstop_t startstop;
spin_unlock_irq(q->queue_lock);
@@ -485,16 +485,13 @@ repeat:
spin_unlock_irq(&hwif->lock);
spin_lock_irq(q->queue_lock);
+ rq = blk_fetch_request(drive->queue);
+ spin_unlock_irq(q->queue_lock);
+ spin_lock_irq(&hwif->lock);
/*
* we know that the queue isn't empty, but this can happen
* if the q->prep_rq_fn() decides to kill a request
*/
- if (!rq)
- rq = blk_fetch_request(drive->queue);
-
- spin_unlock_irq(q->queue_lock);
- spin_lock_irq(&hwif->lock);
-
if (!rq) {
ide_unlock_port(hwif);
goto out;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ide: no need to check for the rq pointer to be NULL
2011-01-09 23:51 [PATCH] ide: no need to check for the rq pointer to be NULL Vasily Tarasov
@ 2011-01-10 12:25 ` Sergei Shtylyov
2011-01-10 17:30 ` Vasily Tarasov
0 siblings, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2011-01-10 12:25 UTC (permalink / raw)
To: Vasily Tarasov; +Cc: davem, linux-ide
Hello.
On 10-01-2011 2:51, Vasily Tarasov wrote:
> From: Vasily Tarasov<sectorsize512@gmail.com>
> No need to check for the rq pointer to be NULL before calling
> blk_fetch_request(). First we call it, then we test the pointer that it
> returns.
> Signed-off-by: Vasily Tarasov<sectorsize512@gmail.com>
I'm not feeling sure about this patch...
> --- linux-2.6.37/drivers/ide/ide-io.c.orig 2011-01-04 19:50:19.000000000 -0500
> +++ linux-2.6.37/drivers/ide/ide-io.c 2011-01-09 18:15:44.000000000 -0500
> @@ -438,7 +438,7 @@ void do_ide_request(struct request_queue
> ide_drive_t *drive = q->queuedata;
> ide_hwif_t *hwif = drive->hwif;
> struct ide_host *host = hwif->host;
> - struct request *rq = NULL;
> + struct request *rq;
> ide_startstop_t startstop;
>
> spin_unlock_irq(q->queue_lock);
> @@ -485,16 +485,13 @@ repeat:
>
> spin_unlock_irq(&hwif->lock);
> spin_lock_irq(q->queue_lock);
> + rq = blk_fetch_request(drive->queue);
> + spin_unlock_irq(q->queue_lock);
> + spin_lock_irq(&hwif->lock);
> /*
> * we know that the queue isn't empty, but this can happen
> * if the q->prep_rq_fn() decides to kill a request
> */
> - if (!rq)
> - rq = blk_fetch_request(drive->queue);
Did you consider the scenario where the 'goto' is taken at the end of this
*if* branch? 'rq' is non-NULL then...
> -
> - spin_unlock_irq(q->queue_lock);
> - spin_lock_irq(&hwif->lock);
> -
> if (!rq) {
> ide_unlock_port(hwif);
> goto out;
WBR, Sergei
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ide: no need to check for the rq pointer to be NULL
2011-01-10 12:25 ` Sergei Shtylyov
@ 2011-01-10 17:30 ` Vasily Tarasov
2011-01-10 18:03 ` Sergei Shtylyov
0 siblings, 1 reply; 4+ messages in thread
From: Vasily Tarasov @ 2011-01-10 17:30 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: davem, linux-ide
Ooops, you're right, ide_kill_rq() sets hwif->rq (and rq in turn) to NULL.
Please disregard this patch.
Vasily
On Mon, Jan 10, 2011 at 03:25:47PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 10-01-2011 2:51, Vasily Tarasov wrote:
>
> >From: Vasily Tarasov<sectorsize512@gmail.com>
>
> >No need to check for the rq pointer to be NULL before calling
> >blk_fetch_request(). First we call it, then we test the pointer that it
> >returns.
>
> >Signed-off-by: Vasily Tarasov<sectorsize512@gmail.com>
>
> I'm not feeling sure about this patch...
>
> >--- linux-2.6.37/drivers/ide/ide-io.c.orig 2011-01-04 19:50:19.000000000 -0500
> >+++ linux-2.6.37/drivers/ide/ide-io.c 2011-01-09 18:15:44.000000000 -0500
> >@@ -438,7 +438,7 @@ void do_ide_request(struct request_queue
> > ide_drive_t *drive = q->queuedata;
> > ide_hwif_t *hwif = drive->hwif;
> > struct ide_host *host = hwif->host;
> >- struct request *rq = NULL;
> >+ struct request *rq;
> > ide_startstop_t startstop;
> >
> > spin_unlock_irq(q->queue_lock);
> >@@ -485,16 +485,13 @@ repeat:
> >
> > spin_unlock_irq(&hwif->lock);
> > spin_lock_irq(q->queue_lock);
> >+ rq = blk_fetch_request(drive->queue);
> >+ spin_unlock_irq(q->queue_lock);
> >+ spin_lock_irq(&hwif->lock);
> > /*
> > * we know that the queue isn't empty, but this can happen
> > * if the q->prep_rq_fn() decides to kill a request
> > */
> >- if (!rq)
> >- rq = blk_fetch_request(drive->queue);
>
> Did you consider the scenario where the 'goto' is taken at the
> end of this *if* branch? 'rq' is non-NULL then...
>
> >-
> >- spin_unlock_irq(q->queue_lock);
> >- spin_lock_irq(&hwif->lock);
> >-
> > if (!rq) {
> > ide_unlock_port(hwif);
> > goto out;
>
> WBR, Serge
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ide: no need to check for the rq pointer to be NULL
2011-01-10 17:30 ` Vasily Tarasov
@ 2011-01-10 18:03 ` Sergei Shtylyov
0 siblings, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2011-01-10 18:03 UTC (permalink / raw)
To: Vasily Tarasov; +Cc: Sergei Shtylyov, davem, linux-ide
Vasily Tarasov wrote:
> Ooops, you're right, ide_kill_rq() sets hwif->rq (and rq in turn) to NULL.
Frankly speaking, I don't see where...
WBR, Serge
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-01-10 18:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-09 23:51 [PATCH] ide: no need to check for the rq pointer to be NULL Vasily Tarasov
2011-01-10 12:25 ` Sergei Shtylyov
2011-01-10 17:30 ` Vasily Tarasov
2011-01-10 18:03 ` Sergei Shtylyov
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.