El Wed, 19 May 2010 10:37:21 +0200
<olafbuddenha...@gmx.net> escribió:
> On Tue, May 18, 2010 at 04:21:14PM +0200, Sergio Lopez wrote:
>
> > This patch modifies tmpfs to keep a reference (by mapping it into
> > its own space) to each memory object created by the user, so they
> > don't get inmediately terminated at the end of the current
> > operation.
>
> Hm... Do I get it right, that when the client closes a file (or
> otherwise drops the mapping), the object holding the data was getting
> lost? Ouch...
>
Yes, but Mach is doing what we've told him, since the default pager is
creating the object with can_cache=FALSE. Anyway, if we change default
pager to create external objects with can_cache=TRUE, we'll still have
this problem when objects get expeled from cache.
The real problem is that Mach thinks that every object can be recreated
at any time. This is true for a translator like ext2fs, as we can
locate the contents of a file a provide its contents to Mach every time
we're requested to. But with the default pager, even if its contents
are currently written to swap space, if a object is terminated we lost
all our connections with actual data.
I think that, in future, in parallel with a new cache policy for memory
objects, we should implement a special treatment for named memory
objects without a backing store.
> (The comment in the code says something slightly different though. So
> it doesn't happen immediately, only when cleaning the cache?)
>
You're right, I've wrote that comment when I was testing it with a
default pager which created objects with can_cache=TRUE. I've changed
it now.
> > @@ -489,7 +491,7 @@
> > {
> > error_t err = default_pager_object_create (default_pager,
> > &np->dn->u.reg.memobj,
> > - np->allocsize);
> > + vm_page_size);
> > if (err)
> > {
> > errno = err;
>
> Hm... This change doesn't look like it's related to the issue you
> described above... Am I missing something, or did you sneak in an
> unrelated fix here? ;-)
>
Yes, it's unrelated. I've splitted it up in other patch.
> > @@ -500,6 +502,13 @@
> > past the specified size of the file. */
> > err = default_pager_object_set_size (np->dn->u.reg.memobj,
> > np->allocsize);
> > + assert_perror (err);
>
> Again, seems unrelated?...
No, this _is_ related :-). I want to make sure that the call to
default_pager_object_set_size didn't return an error before calling
vm_map().
>
> > + /* XXX we need to keep a reference to the object, or GNU Mach
> > + could try to terminate it while cleaning object cache */
>
> Why the XXX? Do you think this should be fixed another way in the
> long run?
>
Yes, as I wrote above, IMHO there should be a special treatment for
objects without a backing store.
Here is a little explanation of these patches:
- 01tmpfs_objectsize.patch: Create objects with size == vm_page_size,
since the default_pager doesn't properly support other values.
(object size will be virtually increased with successive calls to
default_pager_set_size).
- 02tmpfs_memref.patch: Keep a reference to the memory object. This
is better explained above.
diff -dur a/tmpfs/node.c b/tmpfs/node.c
--- a/tmpfs/node.c 2010-05-19 16:01:43.000000000 +0200
+++ b/tmpfs/node.c 2010-05-19 16:04:27.000000000 +0200
@@ -61,8 +61,10 @@
switch (np->dn->type)
{
case DT_REG:
- if (np->dn->u.reg.memobj != MACH_PORT_NULL)
+ if (np->dn->u.reg.memobj != MACH_PORT_NULL) {
+ vm_deallocate (mach_task_self (), np->dn->u.reg.memref, 4096);
mach_port_deallocate (mach_task_self (), np->dn->u.reg.memobj);
+ }
break;
case DT_DIR:
assert (np->dn->u.dir.entries == 0);
@@ -500,6 +502,13 @@
past the specified size of the file. */
err = default_pager_object_set_size (np->dn->u.reg.memobj,
np->allocsize);
+ assert_perror (err);
+
+ /* XXX we need to keep a reference to the object, or GNU Mach
+ will terminate it when we release the map. */
+ vm_map (mach_task_self (), &np->dn->u.reg.memref, 4096, 0, 1,
+ np->dn->u.reg.memobj, 0, 0, VM_PROT_NONE, VM_PROT_NONE,
+ VM_INHERIT_NONE);
}
/* XXX always writable */
diff -dur a/tmpfs/tmpfs.h b/tmpfs/tmpfs.h
--- a/tmpfs/tmpfs.h 2010-05-19 15:51:44.000000000 +0200
+++ b/tmpfs/tmpfs.h 2010-05-19 16:00:03.000000000 +0200
@@ -47,6 +47,7 @@
struct
{
mach_port_t memobj;
+ vm_address_t memref;
unsigned int allocpages; /* largest size while memobj was live */
} reg;
struct
diff -dur a/tmpfs/node.c b/tmpfs/node.c
--- a/tmpfs/node.c 2010-05-19 15:51:44.000000000 +0200
+++ b/tmpfs/node.c 2010-05-19 15:53:47.000000000 +0200
@@ -489,7 +489,7 @@
{
error_t err = default_pager_object_create (default_pager,
&np->dn->u.reg.memobj,
- np->allocsize);
+ vm_page_size);
if (err)
{
errno = err;
