El Wed, 19 May 2010 10:17:00 +0200
<[email protected]> escribió:
> On Tue, May 18, 2010 at 04:19:10PM +0200, Sergio Lopez wrote:
>
> > This patch fixes external objects interface in mach_defpager
> > (current default pager in Hurd), so it can be used as backing store
> > by other translators (like tmpfs).
>
> I don't know enough about this code to actually understand the
> changes. However, on a quick glance, there seem to be several
> unrelated fixes in this patch?... Please split them into separate
> patches; and add a short explanation to each.
>
OK, here we go:
- 01defpager-mutex.patch: Properly unlock the mutex before returning
NO_BLOCK in pager_read_offset.
- 02defpager-extread.patch: Add an "external" attribute to
dstruct structure. In default_read, if the object is external,
resolve a read request with a zero filled page.
- 03defpager-synclock.patch: Remove user reference count logic, to
avoid setting an arbitrary limit to the number of requests
processed by an object. Don't request completion notification for
m_o_lock_request. I think is worth noting that the original problem
which makes default_pager get stuck waiting for a seqno that never
arrives in S_default_pager_object_set_size, was just that
reply_port and seqno arguments were in wrong order.
- 04defpager-objectsize.patch: In S_default_pager_object_create,
reject values for "size" other than "vm_page_size", since internal
logic doesn't properly support it (though object size limit can be
increased by sucesive calls to S_default_pager_set_size).
--- a/serverboot/default_pager.c 2010-05-19 15:47:03.000000000 +0200
+++ b/serverboot/default_pager.c 2010-05-19 15:48:03.000000000 +0200
@@ -3251,7 +3251,8 @@
mach_port_t port;
kern_return_t result;
- if (pager != default_pager_default_port)
+ if (pager != default_pager_default_port ||
+ size != vm_page_size)
return KERN_INVALID_ARGUMENT;
ds = pager_port_alloc(size);
--- a/serverboot/default_pager.c 2010-05-19 13:37:53.000000000 +0200
+++ b/serverboot/default_pager.c 2010-05-19 15:27:33.000000000 +0200
@@ -2259,39 +2259,6 @@
mach_port_urefs_t default_pager_max_urefs = 10000;
-/*
- * Check user reference count on pager_request port.
- * Pager must be locked.
- * Unlocks and re-locks pager if needs to call kernel.
- */
-void pager_port_check_request(ds, pager_request)
- default_pager_t ds;
- mach_port_t pager_request;
-{
- mach_port_delta_t delta;
- kern_return_t kr;
-
- assert(ds->pager_request == pager_request);
-
- if (++ds->request_refs > default_pager_max_urefs) {
- delta = 1 - ds->request_refs;
- ds->request_refs = 1;
-
- dstruct_unlock(ds);
-
- /*
- * Deallocate excess user references.
- */
-
- kr = mach_port_mod_refs(default_pager_self, pager_request,
- MACH_PORT_RIGHT_SEND, delta);
- if (kr != KERN_SUCCESS)
- panic("%spager_port_check_request",my_name);
-
- dstruct_lock(ds);
- }
-}
-
void default_pager_add(ds, internal)
default_pager_t ds;
boolean_t internal;
@@ -2512,28 +2479,11 @@
pager_port_unlock(ds);
/*
- * Now we deallocate our various port rights.
+ * Now we destroy our port rights.
*/
- kr = mach_port_mod_refs(default_pager_self, pager_request,
- MACH_PORT_RIGHT_SEND, -request_refs);
- if (kr != KERN_SUCCESS)
- panic(here,my_name);
-
- kr = mach_port_mod_refs(default_pager_self, pager_request,
- MACH_PORT_RIGHT_RECEIVE, -1);
- if (kr != KERN_SUCCESS)
- panic(here,my_name);
-
- kr = mach_port_mod_refs(default_pager_self, pager_name,
- MACH_PORT_RIGHT_SEND, -name_refs);
- if (kr != KERN_SUCCESS)
- panic(here,my_name);
-
- kr = mach_port_mod_refs(default_pager_self, pager_name,
- MACH_PORT_RIGHT_RECEIVE, -1);
- if (kr != KERN_SUCCESS)
- panic(here,my_name);
+ mach_port_destroy(mach_task_self(), pager_request);
+ mach_port_destroy(mach_task_self(), pager_name);
return (KERN_SUCCESS);
}
@@ -2642,7 +2592,6 @@
ddprintf ("seqnos_memory_object_data_request <%p>: pager_port_lock: <%p>[s:%d,r:%d,w:%d,l:%d], %d\n",
&ds, ds, ds->seqno, ds->readers, ds->writers, ds->lock.held, seqno);
pager_port_lock(ds, seqno);
- pager_port_check_request(ds, reply_to);
pager_port_wait_for_writers(ds);
pager_port_start_read(ds);
@@ -2745,7 +2694,6 @@
ddprintf ("seqnos_memory_object_data_initialize <%p>: pager_port_lock: <%p>[s:%d,r:%d,w:%d,l:%d], %d\n",
&ds, ds, ds->seqno, ds->readers, ds->writers, ds->lock.held, seqno);
pager_port_lock(ds, seqno);
- pager_port_check_request(ds, pager_request);
pager_port_start_write(ds);
ddprintf ("seqnos_memory_object_data_initialize <%p>: pager_port_unlock: <%p>[s:%d,r:%d,w:%d,l:%d]\n",
&ds, ds, ds->seqno, ds->readers, ds->writers, ds->lock.held);
@@ -2825,7 +2773,6 @@
&err, ds, ds->seqno, ds->readers, ds->writers, ds->lock.held, seqno);
pager_port_lock(ds, seqno);
ddprintf ("seqnos_memory_object_data_write <%p>: 5\n", &err);
- pager_port_check_request(ds, pager_request);
ddprintf ("seqnos_memory_object_data_write <%p>: 6\n", &err);
pager_port_start_write(ds);
ddprintf ("seqnos_memory_object_data_write <%p>: 7\n", &err);
@@ -2893,8 +2840,6 @@
return KERN_FAILURE;
}
-/* We get this when our memory_object_lock_request has completed
- after we truncated an object. */
kern_return_t
seqnos_memory_object_lock_completed (memory_object_t pager,
mach_port_seqno_t seqno,
@@ -2902,30 +2847,8 @@
vm_offset_t offset,
vm_size_t length)
{
- default_pager_t ds;
-
- ds = pager_port_lookup(pager);
- assert(ds != DEFAULT_PAGER_NULL);
-
- pager_port_lock(ds, seqno);
- pager_port_wait_for_readers(ds);
- pager_port_wait_for_writers(ds);
-
- /* Now that any in-core pages have been flushed, we can apply
- the limit to prevent any new page-ins. */
- assert (page_aligned (offset));
- ds->dpager.limit = offset;
-
- default_pager_object_set_size_reply (ds->lock_request, KERN_SUCCESS);
- ds->lock_request = MACH_PORT_NULL;
-
- if (ds->dpager.size > ds->dpager.limit / vm_page_size)
- /* Deallocate the old backing store pages and shrink the page map. */
- pager_truncate (&ds->dpager, ds->dpager.limit / vm_page_size);
-
- pager_port_unlock(ds, seqno);
-
- return KERN_SUCCESS;
+ panic("%slock_completed", my_name);
+ return KERN_FAILURE;
}
kern_return_t
@@ -3725,7 +3648,6 @@
kern_return_t
S_default_pager_object_set_size (mach_port_t pager,
mach_port_seqno_t seqno,
- mach_port_t reply_to,
vm_size_t limit)
{
kern_return_t kr;
@@ -3736,7 +3658,6 @@
return KERN_INVALID_ARGUMENT;
pager_port_lock(ds, seqno);
- pager_port_check_request(ds, reply_to);
pager_port_wait_for_readers(ds);
pager_port_wait_for_writers(ds);
@@ -3747,26 +3668,23 @@
ds->dpager.limit = limit;
kr = KERN_SUCCESS;
}
- else if (ds->lock_request == MACH_PORT_NULL)
+ else
{
- /* Tell the kernel to flush from core all the pages being removed.
- We will get the memory_object_lock_completed callback when they
- have been flushed. We handle that by completing the limit update
- and posting the reply to the pending truncation. */
+ /* Tell the kernel to flush from core all the pages being removed. */
kr = memory_object_lock_request (ds->pager_request,
limit,
- ds->dpager.size * vm_page_size - limit,
+ ds->dpager.limit - limit,
MEMORY_OBJECT_RETURN_NONE, TRUE,
- VM_PROT_ALL, ds->pager);
+ VM_PROT_ALL, MACH_PORT_NULL);
if (kr != KERN_SUCCESS)
panic ("memory_object_lock_request: %d", kr);
- ds->lock_request = reply_to;
- kr = MIG_NO_REPLY;
+ ds->dpager.limit = limit;
+#if 0
+ /* FIXME: Currently, we're unable to free used swap space */
+ if (ds->dpager.size > ds->dpager.limit / vm_page_size)
+ pager_truncate (&ds->dpager, ds->dpager.limit / vm_page_size);
+#endif
}
- else
- /* There is already another call in progress. Tough titties. */
- kr = KERN_FAILURE;
-
pager_port_unlock(ds, seqno);
return kr;
--- a/hurd/default_pager.defs 2010-05-19 15:40:38.000000000 +0200
+++ b/hurd/default_pager.defs 2010-05-19 15:40:53.000000000 +0200
@@ -91,6 +91,5 @@
will fail. */
routine default_pager_object_set_size(
memory_object : mach_port_t;
- sreplyport reply_port : mach_port_send_once_t;
msgseqno seqno : mach_port_seqno_t;
object_size_limit : vm_size_t);
--- a/serverboot/default_pager.c 2010-05-19 15:33:02.000000000 +0200
+++ b/serverboot/default_pager.c 2010-05-19 15:32:20.000000000 +0200
@@ -1154,6 +1154,7 @@
{
ddprintf ("%spager_read_offset pager %x: bad page %d >= size %d",
my_name, pager, f_page, pager->size);
+ mutex_unlock(&pager->lock);
return (union dp_map) (union dp_map *) NO_BLOCK;
#if 0
panic("%spager_read_offset",my_name);
--- a/serverboot/default_pager.c 2010-05-19 13:36:30.000000000 +0200
+++ b/serverboot/default_pager.c 2010-05-19 13:36:39.000000000 +0200
@@ -1668,7 +1668,7 @@
* if it is different from <addr>, it must be deallocated after use.
*/
int
-default_read(ds, addr, size, offset, out_addr, deallocate)
+default_read(ds, addr, size, offset, out_addr, deallocate, external)
register dpager_t ds;
vm_offset_t addr; /* pointer to block to fill */
register vm_size_t size;
@@ -1676,6 +1676,7 @@
vm_offset_t *out_addr;
/* returns pointer to data */
boolean_t deallocate;
+ boolean_t external;
{
register union dp_map block;
vm_offset_t raddr;
@@ -1692,8 +1693,18 @@
* Find the block in the paging partition
*/
block = pager_read_offset(ds, offset);
- if ( no_block(block) )
+ if ( no_block(block) ) {
+ if (external) {
+ /*
+ * An external object is requesting unswapped data,
+ * zero fill the page and return.
+ */
+ bzero((char *) addr, vm_page_size);
+ *out_addr = addr;
+ return (PAGER_SUCCESS);
+ }
return (PAGER_ABSENT);
+ }
/*
* Read it, trying for the entire page.
@@ -1844,6 +1855,7 @@
mach_port_urefs_t request_refs; /* Request port user-refs */
mach_port_t pager_name; /* Name port */
mach_port_urefs_t name_refs; /* Name port user-refs */
+ boolean_t external; /* Is an external object? */
unsigned int readers; /* Reads in progress */
unsigned int writers; /* Writes in progress */
@@ -2301,10 +2313,12 @@
/* possibly generate an immediate no-senders notification */
sync = 0;
pset = default_pager_internal_set;
+ ds->external = FALSE;
} else {
/* delay notification till send right is created */
sync = 1;
pset = default_pager_external_set;
+ ds->external = TRUE;
}
kr = mach_port_request_notification(default_pager_self, pager,
@@ -2655,7 +2669,8 @@
else
rc = default_read(&ds->dpager, dpt->dpt_buffer,
vm_page_size, offset,
- &addr, protection_required & VM_PROT_WRITE);
+ &addr, protection_required & VM_PROT_WRITE,
+ ds->external);
switch (rc) {
case PAGER_SUCCESS:
