Alle 05:00, gioved? 19 agosto 2004, Jeff Dike ha scritto:
> I've released a second 2.6.7 UML patch. This is to push out the changes I
> have in order to give me a clean slate for the 2.6.8.1 UML.
About the patch (and even the 2.6.8.1-1 one), there are two problems:
* First, please do a "make clean" before releasing the patch. There are some
binaries included in it! And also semaphore.c, which is a symlink normally.
* Second, why do you disable module support when compiling it, or anyhow how
could you succeed to build it? Starting from this patch (this bug is not
there in 2.6.7-1, and remains in 2.6.8.1-1) we have this line twice:
EXPORT_SYMBOL(os_ioctl_generic);
So it did not compile for me (I patched it, obviously). Patch attached -
uml-dup-sym.
Also, you must still export a tons of symbols, plus make hostfs depend on
externfs. Also, to avoid linking against libgcc_s.so and exporting some of
its symbols, which change, I made use of do_div for 64-bit division. For
this, see uml-export-Symbols.patch. It's only for 2.6 - for 2.4 it's a bit
more complex (a module export all its symbols in 2.4, but if you link
statically the code you must export the symbol by hand inside an EXPORT_OBJ;
and if you export a missing symbol you get a link time failure).
Btw, about the ->statfs op: you are missing some unsigned-ness for some
params, since sector_t, used in kstatfs, is unsigned. Do you want them fixed?
* About filehandle_switch: you deleted a line (probably by mistake). Reread
more carefully the separate patches you get with quilt: when you see the
other attached patch (uml-restore-lost-code.patch), you'll agree with me.
Also, what you say about the patch is not correct: filehandle_switch has
almost just a cosmetic effect (there is a change from os_open_file to
open_file for new_mm mode, and nothing else). I've attached the 2.4.26-2 part
which is more actually the filehandle_switch part (it's not a perfect one, it
contains some unrelated changes, but anyway you can fix it).
However, IMHO, since you cannot close and reopen a pipe, it's braindead that
the switch_pipe[] array is an array of filehandles. You must obviously use
the make_pipe() API to call reclaim_fds() if needed, but making it return
filehandles is useless. They are never added onto the list, also, so they
never become reclaimable. But about the filehandle abstraction, I have a lot
of doubts, for which I'll write a separate mail. I like the idea, but not the
current implementation.
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
[email protected] said:
> * First, please do a "make clean" before releasing the patch. There
> are some binaries included in it! And also semaphore.c, which is a
> symlink normally.
I do. It's just that make clean didn't catch everything.
> * About filehandle_switch: you deleted a line (probably by mistake).
> Reread more carefully the separate patches you get with quilt: when
> you see the other attached patch (uml-restore-lost-code.patch),
> you'll agree with me.
Yuck, I have no idea how that happened.
> However, IMHO, since you cannot close and reopen a pipe, it's
> braindead that the switch_pipe[] array is an array of filehandles.
Yeah, this is fixed in my 2.6 tree now.
Jeff
On Thursday 09 September 2004 02:35, Jeff Dike wrote:
> [email protected] said:
> > * First, please do a "make clean" before releasing the patch. There
> > are some binaries included in it! And also semaphore.c, which is a
> > symlink normally.
>
> I do. It's just that make clean didn't catch everything.
Btw, inside patch-scripts they provide a script which rather than diffing two
trees, calls "combinediff" (from patchutils) to merge the patches statically,
without need of the patched files. I've been very confortable with it -
doesn't quilt have something such?
About patchutils (quoting from Andrew Morton):
See http://cyberelk.net/tim/patchutils/ (Don't download the
"experimental" patchutils - it seems to only have half of the
commands in it. Go for "stable")
> > * About filehandle_switch: you deleted a line (probably by mistake).
> > Reread more carefully the separate patches you get with quilt: when
> > you see the other attached patch (uml-restore-lost-code.patch),
> > you'll agree with me.
> Yuck, I have no idea how that happened.
Btw, I'm assuming that you didn't want to drop the HPPFS compile line in
"externfs" (since that's not documented), right?
--- um.orig/fs/Makefile 2004-08-06 15:17:22.000000000 -0400
+++ um/fs/Makefile 2004-08-06 15:17:25.000000000 -0400
@@ -91,5 +91,4 @@
obj-$(CONFIG_XFS_FS) += xfs/
obj-$(CONFIG_AFS_FS) += afs/
obj-$(CONFIG_BEFS_FS) += befs/
-obj-$(CONFIG_HOSTFS) += hostfs/
-obj-$(CONFIG_HPPFS) += hppfs/ # <---- WHY?
+obj-$(CONFIG_EXTERNFS) += hostfs/
> > However, IMHO, since you cannot close and reopen a pipe, it's
> > braindead that the switch_pipe[] array is an array of filehandles.
> Yeah, this is fixed in my 2.6 tree now.
Yes, I saw it, a lot after writing the message (I sent it a lot after writing
it).
However, another thing: I think that the handling of EMFILE/ENFILE (too many
fd's for the app or for the system) should be moved inside the os_ layer. Or
will you create yet a filehandle wrapper for functions like
os_connect_socket() (which calls socket(), which requests an fd)? Do you
agree or have any arguments to support the current design?
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729