Re: suspend.c: This is broken, fixme

[Jens Axboe <axboe@suse.de>]

On Mon, Jun 03 2002, Pavel Machek wrote:
> Hi!
> 
> > > @@ -300,7 +301,8 @@
> > >  static void do_suspend_sync(void)
> > >  {
> > >         while (1) {
> > > -               run_task_queue(&tq_disk);
> > > +               blk_run_queues();
> > > +#error this is broken, FIXME
> > >                 if (!TQ_ACTIVE(tq_disk))
> > >                         break;
> > > 
> > > . Why is it broken?
> > 
> > Hey, I even cc'ed you on the patch when it went to Linus... Lets
> > look at
> 
> Okay; I thought I corrected it in the meantime, that's why I got confused.
> 
> > what happened before: run tq_disk, then check if it is active. What
> > prevents tq_disk from being active right after you issue the TQ_ACTIVE
> > check? Nothing. And I'm not sure exactly what semantics you think
> > running tq_disk has. I suspect you are looking for a 'start any pending
> > i/o and return when it has completed', which is far from what happens.
> > Running tq_disk will _try_ to start _some_ I/O, and eventually, in time,
> > the currently pending requests will have completed. In the mean time,
> > more I/O could have been added though.
> 
> I'm alone at the system at that point. All user tasks are stopped and
> I'm only thread running. There's noone that could submit requests at
> that point.

Ok, then at least the very last point I made can be disregarded.
However... ->

> In such case, killing #error is right solution, right?

Not at all. The tq_disk/blk_run_queues() semantics are the same, they
will only start i/o (which may not even be right when you run it) and
that is it. When all i/o is completed is not known.

-- 
Jens Axboe