New: pktgen procfs problem

Bugme-new Andrew Morton 6 participants 13 messages May 17, 2008 – May 20, 2008 1 message not in the archive

Subsystems (3)

Networking Network drivers Proc filesystem

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]/

Andrew Morton

author

2008-05-17 21:11 UTC #1 lore raw

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 25 lines · On Sat, 17 May 2008 13:59:25 -0700 (PDT) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=10737
           Summary: pktgen procfs problem
           Product: Networking
           Version: 2.5
     KernelVersion: 2.6.26-rc2
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
        AssignedTo: [email protected]
        ReportedBy: [email protected]
Latest working kernel version: n/a
Earliest failing kernel version: 2.6.26-rc2
Distribution: suse 10.1
Hardware Environment: p4 + gigabyte i915g MoBo Software Environment:
Problem Description:
pktgen module ocaasional seems to leak procfs entry

Quoted 4 lines

May 18 00:00:34 test kernel: [ 2663.373955] pktgen v2.69: Packet Generator for
packet performance testing.
May 18 00:00:34 test kernel: [ 2663.384819] remove_proc_entry: removing

non-empty directory 'net/pktgen', leaking at least 'kpktgend_1'

Possibly we were always leaking this procfs entry, and newly-added procfs diagnostics are now detecting it.

Quoted 51 lines

May 18 00:00:34 test kernel: [ 2663.384819] ------------[ cut here
]------------
May 18 00:00:34 test kernel: [ 2663.384819] WARNING: at fs/proc/generic.c:799
remove_proc_entry+0x110/0x12d()
May 18 00:00:34 test kernel: [ 2663.384819] Modules linked in: pktgen(-) ip_gre
lzo lzo_decompress lzo_compress tgr192 cts seed camellia fcrypt anubis cast6 cast5 ccm gcm ctr lrw gf128mul aes_i586 aes_generic blowfish md4 cbc ecb sha1_generic aead 8250_accent 8250_exar_st16c554 8250_fourport 8250_boca 8250_hub6 ovcamchip i2c_core guillemot adi a3d cobra gameport parkbd deadline_iosched capifs ar7part mtd eql ide_generic siimage aec62xx trm290 alim15x3 hpt34x hpt366 cmd64x cmd640 piix rz1000 slc90e66 jmicron cs5535 cs5530 cs5520 sc1200 triflex ide_pci_generic atiixp pdc202xx_old pdc202xx_new tc86c001 opti621 ns87415 cy82c693 amd74xx sis5513 via82cxxx serverworks it821x it8213 ide_core eni suni zatm uPD98402 atm non_fatal p4_clockmod speedstep_lib freq_table decnet crypto_blkcipher sctp libcrc32c sunrpc psmouse ipv6 vboxdrv mousedev e100 mii intel_agp agpgart usbcore parport_pc parport megaraid_mbox megaraid_mm sd_mod scsi_mod [last unloaded: ax25]
May 18 00:00:34 test kernel: [ 2663.384819] Pid: 31183, comm: modprobe Tainted:
G        W 2.6.26-rc2-git5 #2
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0122aae>]
warn_on_slowpath+0x41/0x6c
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0135c32>] ? up+0x2b/0x2f
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0122f99>] ?
release_console_sem+0x184/0x18c
May 18 00:00:34 test kernel: [ 2663.384819]  [<c02f5f3e>] ?
schedule_timeout+0x16/0x8b
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0123417>] ? printk+0x15/0x17
May 18 00:00:34 test kernel: [ 2663.384819]  [<c01a9a5a>]
remove_proc_entry+0x110/0x12d
May 18 00:00:34 test kernel: [ 2663.384819]  [<c01ac70b>]
proc_net_remove+0xf/0x11
May 18 00:00:34 test kernel: [ 2663.384819]  [<f8a47be7>] pg_cleanup+0x5f/0x90
[pktgen]
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0140899>]
sys_delete_module+0x18e/0x1d9
May 18 00:00:34 test kernel: [ 2663.384819]  [<c016813d>] ?
remove_vma+0x41/0x47
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0168aa7>] ?
do_munmap+0x17d/0x197
May 18 00:00:34 test kernel: [ 2663.384819]  [<c0103811>]
sysenter_past_esp+0x6a/0x91
May 18 00:00:34 test kernel: [ 2663.384819]  [<c02f0000>] ?
pcibios_setup+0xa8/0x2a0
May 18 00:00:34 test kernel: [ 2663.384819]  =======================
May 18 00:00:34 test kernel: [ 2663.384819] ---[ end trace 4351c3133409518a
]---
Steps to reproduce: not sure yet, but i saw this while doing some module load/unload regression test.

Patrick McHardy

2008-05-18 01:40 UTC #2 lore raw

Quoted 37 lines · Andrew Morton

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 25 lines · On Sat, 17 May 2008 13:59:25 -0700 (PDT) [email protected] wrote:
http://bugzilla.kernel.org/show_bug.cgi?id=10737
           Summary: pktgen procfs problem
           Product: Networking
           Version: 2.5
     KernelVersion: 2.6.26-rc2
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
        AssignedTo: [email protected]
        ReportedBy: [email protected]
Latest working kernel version: n/a
Earliest failing kernel version: 2.6.26-rc2
Distribution: suse 10.1
Hardware Environment: p4 + gigabyte i915g MoBo Software Environment:
Problem Description:
pktgen module ocaasional seems to leak procfs entry
Quoted 5 lines
May 18 00:00:34 test kernel: [ 2663.373955] pktgen v2.69: Packet Generator for
packet performance testing.
May 18 00:00:34 test kernel: [ 2663.384819] remove_proc_entry: removing
non-empty directory 'net/pktgen', leaking at least 'kpktgend_1'

Quoted 5 lines

^^
Possibly we were always leaking this procfs entry, and newly-added procfs diagnostics are now detecting it.

I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:

# /tmp/open /proc/net/kpktgen_0

=> refcnt goes to 1

=> refcnt goes to 0

Without ^C and opening the file a second time:

# /tmp/open /proc/net/kpktgen_0

=> refcnt goes to 2 (sometimes also 11)

=> refcnt stays at previous value.

The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.

Andrew Morton

author

2008-05-18 04:57 UTC #3 lore raw

Quoted 61 lines · On Sun, 18 May 2008 03:39:42 +0200 Patrick McHardy <[email protected]> wrote:

Quoted 37 lines · Andrew Morton
(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).
Quoted 25 lines · On Sat, 17 May 2008 13:59:25 -0700 (PDT) [email protected] wrote:
http://bugzilla.kernel.org/show_bug.cgi?id=10737
           Summary: pktgen procfs problem
           Product: Networking
           Version: 2.5
     KernelVersion: 2.6.26-rc2
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Other
        AssignedTo: [email protected]
        ReportedBy: [email protected]
Latest working kernel version: n/a
Earliest failing kernel version: 2.6.26-rc2
Distribution: suse 10.1
Hardware Environment: p4 + gigabyte i915g MoBo Software Environment:
Problem Description:
pktgen module ocaasional seems to leak procfs entry
Quoted 5 lines
May 18 00:00:34 test kernel: [ 2663.373955] pktgen v2.69: Packet Generator for
packet performance testing.
May 18 00:00:34 test kernel: [ 2663.384819] remove_proc_entry: removing
non-empty directory 'net/pktgen', leaking at least 'kpktgend_1'
Quoted 5 lines
^^
Possibly we were always leaking this procfs entry, and newly-added procfs diagnostics are now detecting it.
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0

Quoted 8 lines

=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.

urgh. Is any of this known to be post-2.6.25?

Patrick McHardy

2008-05-18 12:08 UTC #4 lore raw

Quoted 24 lines · Andrew Morton

Quoted 21 lines · On Sun, 18 May 2008 03:39:42 +0200 Patrick McHardy <[email protected]> wrote:
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 2 (sometimes also 11)
^C

Quoted 8 lines

Quoted 5 lines
=> refcnt stays at previous value.
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.
urgh. Is any of this known to be post-2.6.25?

2.6.25 is also affected. I don't know about earlier kernels.

Patrick McHardy

2008-05-18 13:24 UTC #5 lore raw

Quoted 27 lines · Patrick McHardy

Quoted 26 lines · Andrew Morton
Quoted 23 lines · On Sun, 18 May 2008 03:39:42 +0200 Patrick McHardy <[email protected]> wrote:
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.
urgh. Is any of this known to be post-2.6.25?
2.6.25 is also affected. I don't know about earlier kernels.

Some more information: the problem seems to occur only if the file is opened by two different processes.

I'm starting a bisection now.

Patrick McHardy

2008-05-18 15:31 UTC #6 lore raw

Quoted 30 lines · Patrick McHardy

Quoted 30 lines
Quoted 29 lines
Quoted 17 lines
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0
Quoted 9 lines
=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.
urgh. Is any of this known to be post-2.6.25?

Quoted 6 lines

Quoted 1 line
2.6.25 is also affected. I don't know about earlier kernels.
Some more information: the problem seems to occur only if the file is opened by two different processes.
I'm starting a bisection now.

git-bisect identified e9720acd ([NET]: Make /proc/net a symlink on /proc/self/net (v3)) as the guilty commit. I couldn't find the problem in that commit, so someone with a better understanding of how this is supposed to work should look into it.

Denis V. Lunev

2008-05-19 07:54 UTC #7 lore raw

Quoted 36 lines · Patrick McHardy

Quoted 35 lines · Patrick McHardy
Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.
urgh. Is any of this known to be post-2.6.25?
2.6.25 is also affected. I don't know about earlier kernels.
Some more information: the problem seems to occur only if the file is opened by two different processes.

Quoted 7 lines

Quoted 1 line
I'm starting a bisection now.
git-bisect identified e9720acd ([NET]: Make /proc/net a symlink on /proc/self/net (v3)) as the guilty commit. I couldn't find the problem in that commit, so someone with a better understanding of how this is supposed to work should look into it.

could you consider this preliminary patch? It fixes the problem for me and Pavel agrees with it.

The problem is that module_get is called for each file opening while module_put is called only when /proc inode is destroyed. So, lets put module counter if we are dealing with already initialised inode.

fs/proc/inode.c +2 -1

index 6f4e8dc..b08d100 100644
@@ -425,7 +425,8 @@ struct inode *proc_get_inode(struct super_block *sb, unsigned int ino,
 			}
 		}
 		unlock_new_inode(inode);
-	}
+	} else
+	       module_put(de->owner);
 	return inode;
 
 out_ino:

Patrick McHardy

2008-05-19 10:36 UTC #8 lore raw

Quoted 8 lines · Denis V. Lunev

Quoted 5 lines · Patrick McHardy
git-bisect identified e9720acd ([NET]: Make /proc/net a symlink on /proc/self/net (v3)) as the guilty commit. I couldn't find the problem in that commit, so someone with a better understanding of how this is supposed to work should look into it.

Quoted 7 lines

could you consider this preliminary patch? It fixes the problem for me and Pavel agrees with it.
The problem is that module_get is called for each file opening while module_put is called only when /proc inode is destroyed. So, lets put module counter if we are dealing with already initialised inode.

The patch works fine for me, thanks.

Ben Greear

2008-05-20 00:26 UTC #9 lore raw

In reply to Denis V. Lunev

Quoted 15 lines · Denis V. Lunev

could you consider this preliminary patch? It fixes the problem for me and Pavel agrees with it.
The problem is that module_get is called for each file opening while module_put is called only when /proc inode is destroyed. So, lets put module counter if we are dealing with already initialised inode.
------------------------------------------------------------------------
fs/proc/inode.c +0 -0
index 6f4e8dc..b08d100 100644

fs/proc/inode.c struct inode *proc_get_inode(struct super_block *sb, unsigned int ino, · line 432

Quoted 9 lines

fs/proc/inode.c +2 -1

@@ -425,7 +425,8 @@ struct inode *proc_get_inode(struct super_block *sb, unsigned int ino,
 			}
 		}
 		unlock_new_inode(inode);
-	}
+	} else
+	       module_put(de->owner);
 	return inode;
 
 out_ino:

I just tested this and it seems to fix my problem (I applied this to 2.6.25 kernel).

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Eric W. Biederman

Acked-by

2008-05-20 01:20 UTC #10 lore raw

Quoted 8 lines · Ben Greear

Quoted 6 lines · Denis V. Lunev
could you consider this preliminary patch? It fixes the problem for me and Pavel agrees with it.
The problem is that module_get is called for each file opening while module_put is called only when /proc inode is destroyed. So, lets put module counter if we are dealing with already initialised inode.

Acked-by "Eric W. Biederman" <[email protected]>

Quoted 8 lines

fs/proc/inode.c struct inode *proc_get_inode(struct super_block *sb,
Quoted 7 lines
------------------------------------------------------------------------
fs/proc/inode.c +0 -0
index 6f4e8dc..b08d100 100644
@@ -425,7 +425,8 @@ struct inode *proc_get_inode(struct super_block *sb,
unsigned int ino,

Quoted 11 lines

fs/proc/inode.c struct inode *proc_get_inode(struct super_block *sb, unsigned int ino, · line 432
Quoted 8 lines
fs/proc/inode.c +2 -1
 			}
 		}
 		unlock_new_inode(inode);
-	}
+	} else
+	       module_put(de->owner);
 	return inode;
  out_ino:
I just tested this and it seems to fix my problem (I applied this to 2.6.25 kernel).

Looks good and it seems to follow the characterization. proc_lookup -> proc_lookup_de -> proc_get_inode

Will happen each time a dentry is created for a name/inode. Which is much easier now with multiple instances in proc.

It looks like this bug an old bug that has existed since

commit e9543659715602e3180f00a227bb6db34141ac41

Author: Kirill Korotaev <[email protected]>
Date:   Sun Oct 30 15:02:26 2005 -0800

[PATCH] proc: fix of error path in proc_get_inode()

This patch fixes incorrect error path in proc_get_inode(), when module can't be get due to being unloaded. When try_module_get() fails, this function puts de(!) and still returns inode with non-getted de.

There are still unresolved known bugs in proc yet to be fixed:
- proc_dir_entry tree is managed without any serialization
- create_proc_entry() doesn't setup de->owner anyhow, so setting it later manually is inatomic.
- looks like almost all modules do not care whether it's de->owner is set...

Signed-off-by Denis Lunev <[email protected]>

Signed-off-by Kirill Korotaev <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

Eric

Robert Olsson

Acked-by

2008-05-20 08:25 UTC #11 lore raw

Quoted 15 lines · Eric W. Biederman

Quoted 5 lines
Quoted 3 lines · Denis V. Lunev
could you consider this preliminary patch? It fixes the problem for me and Pavel agrees with it.
Acked-by "Eric W. Biederman" <[email protected]>
Quoted 8 lines
fs/proc/inode.c struct inode *proc_get_inode(struct super_block *sb,
Quoted 7 lines
------------------------------------------------------------------------
fs/proc/inode.c +0 -0
index 6f4e8dc..b08d100 100644
@@ -425,7 +425,8 @@ struct inode *proc_get_inode(struct super_block *sb,
unsigned int ino,

Quoted 8 lines

Quoted 8 lines
fs/proc/inode.c struct inode *proc_get_inode(struct super_block *sb, unsigned int ino, · line 432
Quoted 8 lines
fs/proc/inode.c +2 -1
 			}
 		}
 		unlock_new_inode(inode);
-	}
+	} else
+	       module_put(de->owner);
 	return inode;
  out_ino:

Yes it fixes the oops I got as well.

Acked-by Robert Olsson <[email protected]>

Cheers.

					--ro

Eric W. Biederman

2008-05-19 21:40 UTC #12 lore raw

In reply to Patrick McHardy

Quoted 18 lines · Patrick McHardy

Quoted 17 lines · Patrick McHardy
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.

How are you reading the refcount on kpktgen_0? Just a printk in the kernel code?

Quoted 10 lines

Quoted 4 lines
Some more information: the problem seems to occur only if the file is opened by two different processes.
I'm starting a bisection now.
git-bisect identified e9720acd ([NET]: Make /proc/net a symlink on /proc/self/net (v3)) as the guilty commit. I couldn't find the problem in that commit, so someone with a better understanding of how this is supposed to work should look into it.

To recap:
- The problem is that we get complaints from remove_proc_entry on unload of the pktgen module.

- The problem appears to only happen when multiple processes open the file.

- The problem only appears after we moved /proc/net into /proc/<pid>/net

The obvious candidate is that we have multiple dcache entries for the same proc inode.

It looks like time to reproduce this and see if we can figure out why kpktgend_1 is still exists in the directory we are removing.

Eric

Ben Greear

2008-05-19 22:20 UTC #13 lore raw

Quoted 42 lines · Eric W. Biederman

Quoted 18 lines · Patrick McHardy
Quoted 17 lines · Patrick McHardy
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
I've been looking into the same problem, without much success so far. The problem appears to affect any /proc/net file, but not files outside of /proc/net, so I'm guessing its net-ns related. A testcase found by Ben Greear is opening the file multiple times:
# /tmp/open /proc/net/kpktgen_0
=> refcnt goes to 1
^C
=> refcnt goes to 0
Without ^C and opening the file a second time:
# /tmp/open /proc/net/kpktgen_0
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
=> refcnt goes to 2 (sometimes also 11)
^C
=> refcnt stays at previous value.
The refcnt even leaks if the file can't be successfully opened, for example because of lacking permissions.
How are you reading the refcount on kpktgen_0? Just a printk in the kernel code?
Quoted 9 lines
Quoted 4 lines
Some more information: the problem seems to occur only if the file is opened by two different processes.
I'm starting a bisection now.
git-bisect identified e9720acd ([NET]: Make /proc/net a symlink on /proc/self/net (v3)) as the guilty commit. I couldn't find the problem in that commit, so someone with a better understanding of how this is supposed to work should look into it.

Quoted 7 lines

To recap:
- The problem is that we get complaints from remove_proc_entry on unload of the pktgen module.
- The problem appears to only happen when multiple processes open the file.
- The problem only appears after we moved /proc/net into /proc/<pid>/net

Just to be clear, the problem I saw with too many refs would not even let me remove a module. They may be the same root problem, but not necessarily.

I also saw the problem on multiple modules with proc/net/ file systems, not just pktgen.

The part about multiple processes opening the file is definitely true with my bug report, but based on email I saw, I'm not sure it is true for the remove_proc_entry problem reported by someone else.

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com