El Wed, 19 May 2010 10:17:00 +0200 <olafbuddenha...@gmx.net> 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: