Re: [PATCH] io stalls (was: -rc7 Re: Linux 2.4.21-rc6)

Chris Mason
2003-06-11T01:44:09Z

On Tue, 2003-06-10 at 21:06, Andrea Arcangeli wrote:

> And I don't think any of your barriers is needed at all, I mean, we only
> need to be careful to clear it right, we don't need to be careful to set
> or read it right when it transits from 0 to 1. And the above seems
> enough to me to get right the clearing.
>

The current form of the patch has way too many barriers. When I first
added them the patch was really different, I left them in because it
seems to be easier to remember to rip them out than add them back ;-)

> > > I'm also unsure what the "waited" logic does, it doesn't seem necessary.
> >
> > Once a process waits once, they are allowed to ignore the q->full flag.
> > This way existing waiters can make progress even when q->full is set.
> > Without the waited check, q->full will never get cleared because the
> > last writer wouldn't proceed until the last writer was gone. I had to
> > make __get_request for the same reason.
>
> __get_request makes perfect sense of course and it's needed, this is not
> the issue, my point about the waited check is that the last writer has
> to get the wakeup (and the wakeup has nothing to do with the waited
> check since waited == 0), and after the wakeup it will get the request
> and it won't re-run the loop, so I don't see why waited is needed.
> Furthmore even if for whatever reason it doesn't get the request, it
> will re-set full to 1 and it'll be still the first to get the wakeup,
> and it will definitely get another wakeup if none request was available.

Ok, I see your point, we don't strictly need the waited check. I had
added it as an optimization at first, so that those who waited once were
not penalized by further queue_full checks.

-chris