Re: [uml-devel] [BUG] host fd leak when using hostfs

[BlaisorBlade <blaisorblade_spam@yahoo.it>]

[-- Attachment #1: Type: text/plain, Size: 3485 bytes --]

Alle 01:27, venerdì 4 giugno 2004, Sven Köhler ha scritto:
> hi,
>
> i mounted the directory /usr/portage from my host into my UML-machine
> (also to /usr/portage). The directory is mounted read-only. After that,
> i did "emerge -upD world" which must have accessed many many files.
> emerge crashed with the exception "too many open files" and indeed, the
> processes inside the UML cannot open any file anymore.
>
> If i do an "lsof" on the host, i see that the kernel-process has still
> opened many many files from /usr/portage - too many files ;-)
>
> So this should be bug in hostfs, so that hostfs doesn't close files on
> the host if they are closed inside the UML. Is there a fix for that
> already?

Try the attached patch (for 2.6 only!), but please be very careful... there 
could be subtle pitfall in the VFS making it have other problems. However 
I've well understood what is the difference between 2.4 and 2.6 which makes 
the difference.

What is your UML version? I got the same thing with 2.6.4 patch adapted onto 
2.6.6 (the 2.6.6-02 distributed on my page).

I was expecting it to happen only onto late 2.4 kernels, due to the 
restructuring of hostfs. Instead I saw it onto a 2.6 kernel. I just tested 
that 2.4.23-2 UML kernel, the last one I have at hand, works.

The fd closing is done here:

static struct super_operations hostfs_sbops = {
        .alloc_inode    = hostfs_alloc_inode,
        .destroy_inode  = hostfs_destroy_inode, //this closes the fd.
        .read_inode     = hostfs_read_inode,
        .statfs         = hostfs_statfs,
};

But .destroy_inode was meant by the VFS to be paired with .alloc_inode, i.e. 
think about memory deallocation, basically. I've been able to see that is in, 
indeed, called when UML is closed regularly (sysrq h(alt), I mean).

Without this patch applied and the root_fs_tomsbrt, I did this test:

for i in `find /mnt/usr/bin`; do cat $i >/dev/null; done

with hostfs mounted on /mnt. I got this:

cat: error 24
init_new_context_skas - new_mm failed, errno = -24

(i.e. EMFILE, too many open files.)

And 1023 open file descriptors in /proc/UMLPID/fd (I have ulimit -n (max fd's) 
1024).

While with the attached patch, I repeated the test onto my slack9.0 rootFs and 
it worked very well!

This patch forces the call to delete_inode, i.e. inode with no associated fds 
are not cached, by setting drop_inode = generic_delete_inode (see 
Documentation/filesystems/vfs.txt about setting of drop_inode, and about 
force_delete(), the approach which was used by 2.4 to do the same thing): the 
relevant kernel code is the "iput()" function and sons, which can be called 
by sys_close if the relevant reference counts go to 0.

Normally it uses generic_drop_inode() -> generic_forget_inode(): 
(inode->i_nlink is the number of hard links to that inode):

       if (!inode->i_nlink)
                generic_delete_inode(inode);
        else
                generic_forget_inode(inode);

, which can actually skip (I don't understand when) to call .delete_inode() 
(which is the function which is actually called, check with "dmesg" inside 
UML). On 2.4 this was done, just in a different way (through .put_inode = 
force_delete(), which is mentioned in that doc). force_delete() had problems 
and went away; nobody put something to replace it.

Bye
-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729










[-- Attachment #2: HostFs-2.6-fd_leak-working.patch --]
[-- Type: text/x-diff, Size: 1047 bytes --]

--- ./fs/hostfs/hostfs_kern.c.fix	2004-05-12 15:52:58.000000000 +0200
+++ ./fs/hostfs/hostfs_kern.c	2004-06-12 16:42:34.000000000 +0200
@@ -284,13 +284,25 @@
 	return(&hi->vfs_inode);
 }
 
+static void hostfs_delete_inode(struct inode *inode)
+{
+	if(HOSTFS_I(inode)->fd != -1) {
+		close_file(&HOSTFS_I(inode)->fd);
+		printk("Closing host fd in .delete_inode\n");
+		HOSTFS_I(inode)->fd = -1;
+	}
+	clear_inode(inode);
+}
+
 static void hostfs_destroy_inode(struct inode *inode)
 {
 	if(HOSTFS_I(inode)->host_filename) 
 		kfree(HOSTFS_I(inode)->host_filename);
 
-	if(HOSTFS_I(inode)->fd != -1) 
+	if(HOSTFS_I(inode)->fd != -1) {
 		close_file(&HOSTFS_I(inode)->fd);
+		printk("Closing host fd in .destroy_inode\n");
+	}
 
 	kfree(HOSTFS_I(inode));
 }
@@ -302,6 +314,8 @@
 
 static struct super_operations hostfs_sbops = { 
 	.alloc_inode	= hostfs_alloc_inode,
+	.drop_inode	= generic_delete_inode,
+	.delete_inode   = hostfs_delete_inode,
 	.destroy_inode	= hostfs_destroy_inode,
 	.read_inode	= hostfs_read_inode,
 	.statfs		= hostfs_statfs,