At Tue, 28 Sep 2004 14:07:52 +0300,
Ognyan Kulev wrote:
>
> 2004-09-28 Ognyan Kulev <[EMAIL PROTECTED]>
>
> * data-request.c (_pager_seqnos_memory_object_data_request):
> When pager state is not NORMAL, shorten exit path.
> When _pager_pagemap_resize fails, add call to
> _pager_allow_termination.
>
> @@ -67,14 +67,16 @@ _pager_seqnos_memory_object_data_request
> if (p->pager_state != NORMAL)
> {
> printf ("pager in wrong state for read\n");
> - _pager_release_seqno (p, seqno);
> - mutex_unlock (&p->interlock);
> - goto allow_term_out;
> + _pager_allow_termination (p);
> + goto release_out;
> }
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.
> 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. (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.)
Here is a new untested patch:
2004-09-28 Neal H. Walfield <[EMAIL PROTECTED]>
Ognyan Kulev <[EMAIL PROTECTED]>
* data-request.c (_pager_seqnos_memory_object_data_request):
Elide gratuitous mutex_unlock, mutex_lock sequence.
When _pager_pagemap_resize fails, clean up correctly.
Index: data-request.c
===================================================================
RCS file: /cvsroot/hurd/hurd/libpager/data-request.c,v
retrieving revision 1.22
diff -u -p -r1.22 data-request.c
--- data-request.c 8 May 2002 09:22:14 -0000 1.22
+++ data-request.c 28 Sep 2004 15:03:43 -0000
@@ -1,5 +1,5 @@
/* Implementation of memory_object_data_request for pager library
- Copyright (C) 1994,95,96,97,2000,02 Free Software Foundation
+ Copyright (C) 1994,95,96,97,2000,02,04 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License as
@@ -40,11 +40,11 @@ _pager_seqnos_memory_object_data_request
if (!p)
return EOPNOTSUPP;
- /* Acquire the right to meddle with the pagemap */
+ /* Acquire the right to meddle with the pagemap. */
mutex_lock (&p->interlock);
_pager_wait_for_seqno (p, seqno);
- /* sanity checks -- we don't do multi-page requests yet. */
+ /* Sanity checks -- we don't do multi-page requests yet. */
if (control != p->memobjcntl)
{
printf ("incg data request: wrong control port\n");
@@ -68,13 +68,16 @@ _pager_seqnos_memory_object_data_request
{
printf ("pager in wrong state for read\n");
_pager_release_seqno (p, seqno);
- mutex_unlock (&p->interlock);
goto allow_term_out;
}
err = _pager_pagemap_resize (p, offset + length);
if (err)
- goto release_out; /* Can't do much about the actual error. */
+ {
+ /* Can't do much about the actual error. */
+ _pager_release_seqno (p, seqno);
+ goto allow_term_out;
+ }
/* If someone is paging this out right now, the disk contents are
unreliable, so we have to wait. It is too expensive (right now) to
@@ -109,10 +112,12 @@ _pager_seqnos_memory_object_data_request
/* Let someone else in. */
_pager_release_seqno (p, seqno);
- mutex_unlock (&p->interlock);
if (!doread)
goto allow_term_out;
+
+ mutex_unlock (&p->interlock);
+
if (doerror)
goto error_read;
@@ -133,8 +138,8 @@ _pager_seqnos_memory_object_data_request
error_read:
memory_object_data_error (p->memobjcntl, offset, length, EIO);
_pager_mark_object_error (p, offset, length, EIO);
- allow_term_out:
mutex_lock (&p->interlock);
+ allow_term_out:
_pager_allow_termination (p);
mutex_unlock (&p->interlock);
ports_port_deref (p);
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://lists.gnu.org/mailman/listinfo/bug-hurd
