bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tmpfs: now working


From: Roland McGrath
Subject: Re: [PATCH] tmpfs: now working
Date: Sun, 15 Apr 2001 18:35:16 -0400 (EDT)

Thanks very much for working on this.


>       * tmpfs.c (main): Do not deallocate the underlying node;
>       servers deallocate nodes based only on the number of outstanding
>       protids.

What is this supposed to be about?  There is no protocol requirement that a
filesystem keep a send right to the underlying node.  Since diskfs doesn't
have any use for it, it doesn't keep it.  You shouldn't leak the right if
you aren't storing it anywhere.

>       (diskfs_node_iterate):  Check the return of alloca.

alloca cannot fail; it is inlined to just adjusting the stack pointer register.
If there is a possibility that the stack space of the thread will be too
small, then you have to just not use alloca.

>       (diskfs_truncate): We can truncate objects when they are being
>       truncated to a size of zero.  

Nope.  That was my first thought too--when I wrote that code in the first
place, it looked exactly like yours--but on further consideration I
realized it won't do.  The reason is that there may have been a previous
io_map call (e.g. mmap) that returned the existing memory object.  If a
user has a shared mapping of the file, then that must be valid through the
file being truncated to zero and then enlarged.  In fact, I believe (I'm
not bothering to check POSIX even though the book is lying in front of me)
the user is guaranteed that he can do:

        write(file, "data", size)
        ptr = mmap(file, 0, size)
        access ptr -- see "data"
        truncate(file) // could be ftruncate or via a wholly different peropen
        access ptr -- see SEGV(BUS?) in a specified manner
        write(file, "newdata", size)
        access ptr -- see "newdata"

So you can't "orphan" any previous memory object.  What you could do is
keep track of whether the memory object has ever been passed to the user,
and deallocate the object only if it never has.  (Note that tmpfs doesn't
do the right thing for the second access, which should fault.)

What I'd always intended was to add to the default_pager_object protocol
some calls to make fixed-size objects and explicitly reset their size.
Currently all objects auto-extend, so you can mmap a tmpfs file of size 1
and then write into the second page with no fault (furthermore, a later
extension of the file will think that space is new and not zero it).


I have applied the rest of your changes aside from the ones I've cited above.


>                                     Set the new np->dn_stat.st_size.

Please observe the GNU convention for comments and change log entries,
which is to upcase the names of local variables (i.e. NP->dn_stat).  You
might notice that I have fixed up some of your change log entries in the past.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]