public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Marcin Dalecki <dalecki@evision.ag>
To: Jens Axboe <axboe@suse.de>
Cc: Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>,
	martin@dalecki.de, linux-kernel@vger.kernel.org
Subject: Re: please DON'T run 2.5.27 with IDE!
Date: Wed, 24 Jul 2002 15:35:58 +0200	[thread overview]
Message-ID: <3D3EAD3E.4080800@evision.ag> (raw)
In-Reply-To: 20020724132529.GD15201@suse.de

>>Naj - it's far more trivial I just looked at wrong tree at hand...
>>But anyway. What happens if somone does set QUEUE_FLAG_STOPPED
>>between the test_and_claer_bit and taking the spin_lock? Setting
>>the QUEUE_FLAG_STOPPED isn't maintaining the spin_lock protection!
> 
> 
> It doesn't matter. If QUEUE_FLAG_STOPPED was set when entering
> blk_start_queue(), it will call into the request_fn. If blk_stop_queue()
> is called between clearing QUEUE_FLAG_STOPPED in blk_start_queue() and
> grabbing the spin_lock, the worst that can happen is a spurios extra
> request_fn call.
> 
> 
>>My goal is to make sure that the QUEUE_FLAG_STOPPED has a valid value
>>*inside* the q->request_fn call.
> 
> 
> So you want the queue_lock to protect the flags as well... I don't
> really see the point of this.

Well - OK it's maybe not obvious. So let me please explain: What I have 
in mind is...

1. It doesn't harm and it's a matter of completeness ...
(brain -pedantic)

2. QUEUE_FLAG_STOPPED would suddenly do the same trick as IDE_BUSY does
now and I could just do blk_start_queue() in timout and IRQ handlers in 
IDE. This would bring the "driver in question" in line with all the 
other drivers out there, which indeed do just that instead of explicite
recurrsion in to the request handler...

3. The while(test_and_set_bit(IDE_BUSY, ... ) on do_ide_request entry
could simply go away... and we would have just do_request() left.

4. I worry a bit how this interacts with tcq.c

5. I observed the BUG() during transfers running from one queue to
another comented as by you:

  /* There's a small window between where the queue could be
   * replugged while we are in here when using tcq (in which case
   * the queue is probably empty anyways...), so check and leave
   * if appropriate. When not using tcq, this is still a severe
   * BUG!
   */

in do_request() on a system with enabled preemption and without TCQ
enabled. And I think that the above would plug the possibility of it to 
happen. (Tought here I have to think harder.)


  parent reply	other threads:[~2002-07-24 13:38 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-07-22 19:37 please DON'T run 2.5.27 with IDE! Bartlomiej Zolnierkiewicz
2002-07-22 20:39 ` Andries Brouwer
2002-07-22 23:25   ` Thunder from the hill
2002-07-23  0:39 ` A Guy Called Tyketto
2002-07-23  0:58   ` Thunder from the hill
2002-07-23  1:10     ` Bartlomiej Zolnierkiewicz
2002-07-23  8:03 ` Morten Helgesen
2002-07-23 12:47   ` Bartlomiej Zolnierkiewicz
2002-07-23 13:00     ` Marcin Dalecki
2002-07-23 13:42       ` Bartlomiej Zolnierkiewicz
2002-07-23 13:58         ` Marcin Dalecki
2002-07-23 19:52           ` Jan Harkes
2002-07-23 20:08             ` Andre Hedrick
2002-07-24 10:24             ` Marcin Dalecki
2002-07-23 20:24           ` Bartlomiej Zolnierkiewicz
2002-07-24 10:30             ` Marcin Dalecki
2002-07-24 10:54               ` Bartlomiej Zolnierkiewicz
2002-07-24 11:35                 ` Marcin Dalecki
2002-07-24 11:53                   ` Jens Axboe
2002-07-24 12:08                     ` Marcin Dalecki
2002-07-24 12:39                   ` Bartlomiej Zolnierkiewicz
2002-07-24 12:41                     ` Jens Axboe
2002-07-24 12:49                       ` Bartlomiej Zolnierkiewicz
2002-07-24 12:50                         ` Jens Axboe
2002-07-24 13:08                           ` Marcin Dalecki
2002-07-24 13:25                             ` Jens Axboe
2002-07-24 13:35                               ` Bartlomiej Zolnierkiewicz
2002-07-24 13:36                                 ` Jens Axboe
2002-07-24 13:38                                   ` Marcin Dalecki
2002-07-24 13:35                               ` Marcin Dalecki [this message]
2002-07-24 12:43                   ` Bartlomiej Zolnierkiewicz
2002-07-24 13:10                     ` Marcin Dalecki
2002-07-24 13:21                       ` Bartlomiej Zolnierkiewicz
  -- strict thread matches above, loose matches on Subject: below --
2002-07-22 19:43 Petr Vandrovec
2002-07-22 19:46 ` Bartlomiej Zolnierkiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3D3EAD3E.4080800@evision.ag \
    --to=dalecki@evision.ag \
    --cc=B.Zolnierkiewicz@elka.pw.edu.pl \
    --cc=axboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin@dalecki.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox