Re: [PATCH] libpager/data-request.c: missing call to _pager_allow_termination

[Ognyan Kulev]

Neal H. Walfield wrote:
> This looks correct to me.  Normally, I would say that this sort of
> micro-optimization does not matter as we don't care how slow the error
> path is, however, the other jump to allow_term_out (on the fast path)
> also has a gratuitous unlock relock sequence:
>
>   /* Let someone else in.  */
>   _pager_release_seqno (p, seqno);
>   mutex_unlock (&p->interlock);
>
>   if (!doread)
>     goto allow_term_out;
>
> So, if wold be useful to also move the if above the unlock and moved
> the mutex_lock in allow_term_out to error_read and adjusted callers.

Does this apply to goto error_read too?  Actually, I haven't though that 
unlock-relock is a problem here.

> >    err = _pager_pagemap_resize (p, offset + length);
> >    if (err)
> > -    goto release_out;          /* Can't do much about the actual error.  */
> > +    {
> > +      _pager_allow_termination (p);
> > +      goto release_out;          /* Can't do much about the actual error.  */
> > +    }
>
> This only solves half the problem.  You also have to call
> _pager_release_seqno as well.

It's called in release_out.

  (By the way, this is not the only bug
> involving _pager_pagemap_resize.  data-return.c just assumes that
> _pager_pagemap_resize succeeds.  I think the other callers are,
> however, okay.)

Yes, there are some other problems too, like in pager_offer_page and its use of 
semantics of PM_INCORE.

> 2004-09-28  Neal H. Walfield  <neal@cs.uml.edu>
>         Ognyan Kulev  <ogi@fmi.uni-sofia.bg>

I'm not sure this style is correct.

Regards,
ogi