fix reservation discarding in affs
Subsystems (3)
RFC PATCH 1/1 fix reservation discarding in affs
Currently affs discards preallocations in every ->put_inode but we really want to do this in
a) ->release when a filedescriptor is closed and
b) in ->clear_inode to avoid leaking reservations for shared writeable mappings
affs already does it in a) and a few other places, so adding it toaffs_clear_inode should make it behave fine. Also move the truncate
from affs_put_inode with the racy i_count check to affs_clear_inode. This also avoids the need to take the inode semaphore as the inode can't be accessed from other threads anymore.
fs/affs/inode.c +6 -13
@@ -258,19 +258,6 @@ }void -affs_put_inode (struct inode *inode) -{ -pr_debug ("AFFS: put_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); -affs_free_prealloc (inode); -if (atomic_read (&inode->i_count) ==1 ) { -down (&inode->i_sem); -if (inode->i_size !=AFFS_I (inode)->mmu_private) -affs_truncate (inode); -up (&inode->i_sem); - } -} - -void affs_delete_inode (struct inode *inode) {pr_debug ("AFFS: delete_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink);
@@ -287,6 +274,12 @@unsigned long cache_page = (unsigned long )AFFS_I (inode)->i_lc;pr_debug ("AFFS: clear_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); + +affs_free_prealloc (inode); + +if (inode->i_size !=AFFS_I (inode)->mmu_private) +affs_truncate (inode); +if (cache_page) {pr_debug ("AFFS: freeing ext cache\n" );AFFS_I (inode)->i_lc =NULL ;
===== fs/affs/super.c 1.46 vs edited =====
fs/affs/super.c +0 -1
@@ -133,7 +133,6 @@ .destroy_inode = affs_destroy_inode, .read_inode = affs_read_inode, .write_inode = affs_write_inode, - .put_inode = affs_put_inode, .delete_inode = affs_delete_inode, .clear_inode = affs_clear_inode, .put_super = affs_put_super,
include/linux/affs_fs.h +0 -1
@@ -58,7 +58,6 @@extern unsigned long affs_parent_ino (struct inode *dir);extern struct inode *affs_new_inode (struct inode *dir);extern int affs_notify_change (struct dentry *dentry,struct iattr *attr); -extern void affs_put_inode (struct inode *inode);extern void affs_delete_inode (struct inode *inode);extern void affs_clear_inode (struct inode *inode);extern void affs_read_inode (struct inode *inode);
Quoted 7 lines · Christoph Hellwig
Currently affs discards preallocations in every ->put_inode but we really want to do this in
a) ->release when a filedescriptor is closed and
b) in ->clear_inode to avoid leaking reservations for shared writeable mappings
Quoted 5 lines
affs already does it in a) and a few other places, so adding it to
affs_clear_inodeshould make it behave fine. Also move the truncate
fromaffs_put_inodewith the racyi_countcheck toaffs_clear_inode. This also avoids the need to take the inode semaphore as the inode can't be accessed from other threads anymore.
ping?
fs/affs/inode.c +6 -13
@@ -258,19 +258,6 @@ }void -affs_put_inode (struct inode *inode) -{ -pr_debug ("AFFS: put_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); -affs_free_prealloc (inode); -if (atomic_read (&inode->i_count) ==1 ) { -down (&inode->i_sem); -if (inode->i_size !=AFFS_I (inode)->mmu_private) -affs_truncate (inode); -up (&inode->i_sem); - } -} - -void affs_delete_inode (struct inode *inode) {pr_debug ("AFFS: delete_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink);
@@ -287,6 +274,12 @@unsigned long cache_page = (unsigned long )AFFS_I (inode)->i_lc;pr_debug ("AFFS: clear_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); + +affs_free_prealloc (inode); + +if (inode->i_size !=AFFS_I (inode)->mmu_private) +affs_truncate (inode); +if (cache_page) {pr_debug ("AFFS: freeing ext cache\n" );AFFS_I (inode)->i_lc =NULL ;
===== fs/affs/super.c 1.46 vs edited =====
fs/affs/super.c +0 -1
@@ -133,7 +133,6 @@ .destroy_inode = affs_destroy_inode, .read_inode = affs_read_inode, .write_inode = affs_write_inode, - .put_inode = affs_put_inode, .delete_inode = affs_delete_inode, .clear_inode = affs_clear_inode, .put_super = affs_put_super,
include/linux/affs_fs.h +0 -1
@@ -58,7 +58,6 @@extern unsigned long affs_parent_ino (struct inode *dir);extern struct inode *affs_new_inode (struct inode *dir);extern int affs_notify_change (struct dentry *dentry,struct iattr *attr); -extern void affs_put_inode (struct inode *inode);extern void affs_delete_inode (struct inode *inode);extern void affs_clear_inode (struct inode *inode);extern void affs_read_inode (struct inode *inode);
Hi,
Quoted 7 lines · Christoph Hellwig
Quoted 5 lines
affs already does it in a) and a few other places, so adding it to
affs_clear_inodeshould make it behave fine. Also move the truncate
fromaffs_put_inodewith the racyi_countcheck toaffs_clear_inode. This also avoids the need to take the inode semaphore as the inode can't be accessed from other threads anymore.ping?
Looks fine.
There is still somewhat the problem that I don't want to hold on to the preallocation for ages for directories and very useful would be a callback to flush preallocation when the disk becomes full. Previously I tried it via sync, but maybe someone has an idea how to do this properly.
bye, Roman
Quoted 11 lines · Roman Zippel
Quoted 5 lines
affs already does it in a) and a few other places, so adding it to
affs_clear_inodeshould make it behave fine. Also move the truncate
fromaffs_put_inodewith the racyi_countcheck toaffs_clear_inode. This also avoids the need to take the inode semaphore as the inode can't be accessed from other threads anymore.Looks fine.
There is still somewhat the problem that I don't want to hold on to the preallocation for ages for directories and very useful would be a callback to flush preallocation when the disk becomes full. Previously I tried it via sync, but maybe someone has an idea how to do this properly.
For some reason this never got any traction. Below is a respin against curently mainline. The preallocations are still released later so no progress on that end. Because this is the last instance of ->put_inode I'd like to move forward on getting rid of it.
Is there any chance you could either send me a affs image to run fsx on it or do it yourself?
Signed-off-by Christoph Hellwig <[email protected]>
fs/affs/affs.h +0 -1
@@ -170,7 +170,6 @@ extern int affs_rename(struct inode *oldextern unsigned long affs_parent_ino (struct inode *dir);extern struct inode *affs_new_inode (struct inode *dir);extern int affs_notify_change (struct dentry *dentry,struct iattr *attr); -extern void affs_put_inode (struct inode *inode);extern void affs_drop_inode (struct inode *inode);extern void affs_delete_inode (struct inode *inode);extern void affs_clear_inode (struct inode *inode);
fs/affs/inode.c +1 -6
@@ -239,15 +239,10 @@ out: }void -affs_put_inode (struct inode *inode) +affs_drop_inode (struct inode *inode) { -pr_debug ("AFFS: put_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink);affs_free_prealloc (inode); -} -void -affs_drop_inode (struct inode *inode) -{mutex_lock (&inode->i_mutex);if (inode->i_size !=AFFS_I (inode)->mmu_private)affs_truncate (inode);
fs/affs/super.c +0 -1
@@ -115,7 +115,6 @@ static const struct super_operations aff .destroy_inode = affs_destroy_inode, .read_inode = affs_read_inode, .write_inode = affs_write_inode, - .put_inode = affs_put_inode, .drop_inode = affs_drop_inode, .delete_inode = affs_delete_inode, .clear_inode = affs_clear_inode,
Hi,
Quoted 2 lines · Christoph Hellwig
Is there any chance you could either send me a affs image to run fsx on it or do it yourself?
If you want you can use http://www.xs4all.nl/~zippel/affstools-0.1a.tar.gz
to create one.
bye, Roman
Quoted 9 lines · Roman Zippel
Hi,
Quoted 2 lines · Christoph Hellwig
Is there any chance you could either send me a affs image to run fsx on it or do it yourself?
If you want you can use http://www.xs4all.nl/~zippel/affstools-0.1a.tar.gz
to create one.
Okay, I started playing with that. First mkaffs craps out when you hand it a too large partition and stays in an uninterruptible loop for a long time. Might be worth adding some sanity check then.
Then affs doesn't really survive fsstress from ltp very well. The first lockdep warning I got is from my implement drop_inode patch but even with that things look quite bad, with both lockdep warnings and lots of 'VFS: brelse: Trying to free free buffer' with following stack trace.
Any idea how to proceed with affs?
Feb 6 19:31:01 debian kernel: [ 96.791560] =============================================
Feb 6 19:31:01 debian kernel: [ 96.794749] [ INFO: possible recursive locking detected ]
Feb 6 19:31:01 debian kernel: [ 96.794749] 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:01 debian kernel: [ 96.794749] ---------------------------------------------
Feb 6 19:31:01 debian kernel: [ 96.794749] fsstress/2256 is trying to acquire lock:
Feb 6 19:31:01 debian kernel: [ 96.794749] (&sb->s_type->i_mutex_key#10){--..}, at: [<f083724f>] affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749]
Feb 6 19:31:01 debian kernel: [ 96.794749] but task is already holding lock:
Feb 6 19:31:01 debian kernel: [ 96.794749] (&sb->s_type->i_mutex_key#10){--..}, at: [<c0177077>] vfs_rmdir+0x74/0xd4
Feb 6 19:31:01 debian kernel: [ 96.794749]
Feb 6 19:31:01 debian kernel: [ 96.794749] other info that might help us debug this:
Feb 6 19:31:01 debian kernel: [ 96.794749] 2 locks held by fsstress/2256:
Feb 6 19:31:01 debian kernel: [ 96.794749] #0: (&sb->s_type->i_mutex_key#10/1){--..}, at: [<c017880b>] do_rmdir+0x6d/0xc4
Feb 6 19:31:01 debian kernel: [ 96.794749] #1: (&sb->s_type->i_mutex_key#10){--..}, at: [<c0177077>] vfs_rmdir+0x74/0xd4
Feb 6 19:31:01 debian kernel: [ 96.794749]
Feb 6 19:31:01 debian kernel: [ 96.794749] stack backtrace:
Feb 6 19:31:01 debian kernel: [ 96.794749] Pid: 2256, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c013e265>] __lock_acquire+0x7b4/0xab3
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c013e90e>] lock_acquire+0x57/0x73
Feb 6 19:31:01 debian kernel: [ 96.794749] [<f083724f>] ? affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c03c0c96>] mutex_lock_nested+0xc6/0x21d
Feb 6 19:31:01 debian kernel: [ 96.794749] [<f083724f>] ? affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [<f083724f>] ? affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [<f083724f>] affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017fd54>] iput+0x2f/0x66
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017df31>] dentry_iput+0x7c/0x97
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017dfc5>] d_kill+0x1c/0x36
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017e06c>] prune_one_dentry+0x8d/0xa0
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017e14c>] prune_dcache+0xcd/0x138
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017edc9>] shrink_dcache_parent+0x1c/0xe4
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c0176c31>] dentry_unhash+0x1e/0x72
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017707e>] vfs_rmdir+0x7b/0xd4
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c017882b>] do_rmdir+0x8d/0xc4
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c0170db7>] ? fput+0x17/0x19
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c0103e6d>] ? sysenter_past_esp+0x9a/0xa5
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c013d6ed>] ? trace_hardirqs_on+0xe1/0x102
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c01788a1>] sys_rmdir+0x10/0x12
Feb 6 19:31:01 debian kernel: [ 96.794749] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:01 debian kernel: [ 96.794749] =======================
Feb 6 19:31:38 debian kernel: [ 133.888308] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 133.888308] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 133.888316] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 133.891646] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 133.894737] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 133.894983] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 133.898316] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 133.899925] [<c012289e>] ? wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 133.902495] [<c0122a58>] ? release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 133.904973] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 133.906135] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 133.907652] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 133.908306] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 133.908306] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 133.908306] [<c0269596>] ? copy_to_user+0x3d/0x47
Feb 6 19:31:38 debian kernel: [ 133.908317] [<c017a14b>] ? filldir64+0x97/0xcd
Feb 6 19:31:38 debian kernel: [ 133.911647] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 133.912910] [<f08398f9>] affs_readdir+0x2bd/0x3d0 [affs]
Feb 6 19:31:38 debian kernel: [ 133.915845] [<c017a0b4>] ? filldir64+0x0/0xcd
Feb 6 19:31:38 debian kernel: [ 133.917243] [<c017a298>] vfs_readdir+0x56/0x82
Feb 6 19:31:38 debian kernel: [ 133.918306] [<c017a0b4>] ? filldir64+0x0/0xcd
Feb 6 19:31:38 debian kernel: [ 133.918306] [<c017a322>] sys_getdents64+0x5e/0xa0
Feb 6 19:31:38 debian kernel: [ 133.918989] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 133.920506] =======================
Feb 6 19:31:38 debian kernel: [ 133.921576] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 133.935801] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 133.938304] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 133.938335] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 133.941644] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 133.947194] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c012289e>] ? wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c0122a58>] ? release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 133.948304] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 133.949964] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 133.951254] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 133.951637] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 133.951637] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 133.951637] [<f0836d74>] affs_lookup+0x97/0x241 [affs]
Feb 6 19:31:38 debian kernel: [ 133.952473] [<f0836914>] ? affs_toupper+0x0/0x11 [affs]
Feb 6 19:31:38 debian kernel: [ 133.954093] [<c017634a>] do_lookup+0xa2/0x13f
Feb 6 19:31:38 debian kernel: [ 133.954970] [<c0177bf1>] __link_path_walk+0x721/0xb13
Feb 6 19:31:38 debian kernel: [ 133.955769] [<c012e3e6>] ? in_group_p+0x24/0x26
Feb 6 19:31:38 debian kernel: [ 133.957216] [<c0177548>] ? __link_path_walk+0x78/0xb13
Feb 6 19:31:38 debian kernel: [ 133.958311] [<c017802e>] link_path_walk+0x4b/0xc0
Feb 6 19:31:38 debian kernel: [ 133.959770] [<c016e6ca>] ? get_unused_fd_flags+0xce/0xd8
Feb 6 19:31:38 debian kernel: [ 133.961636] [<c03c1dbc>] ? _spin_unlock+0x1d/0x20
Feb 6 19:31:38 debian kernel: [ 133.961648] [<c01780bc>] path_walk+0x19/0x1b
Feb 6 19:31:38 debian kernel: [ 133.967096] [<c017838a>] do_path_lookup+0x179/0x193
Feb 6 19:31:38 debian kernel: [ 133.968311] [<c0178c84>] __path_lookup_intent_open+0x42/0x74
Feb 6 19:31:38 debian kernel: [ 133.971649] [<c0178d14>] path_lookup_open+0x10/0x12
Feb 6 19:31:38 debian kernel: [ 133.974969] [<c0178dee>] open_namei+0x61/0x512
Feb 6 19:31:38 debian kernel: [ 133.974969] [<c016e8f7>] do_filp_open+0x1f/0x35
Feb 6 19:31:38 debian kernel: [ 133.974969] [<c016e6ca>] ? get_unused_fd_flags+0xce/0xd8
Feb 6 19:31:38 debian kernel: [ 133.974969] [<c03c1dbc>] ? _spin_unlock+0x1d/0x20
Feb 6 19:31:38 debian kernel: [ 133.974969] [<c016e94f>] do_sys_open+0x42/0xbc
Feb 6 19:31:38 debian kernel: [ 133.974981] [<c016e9f5>] sys_open+0x16/0x18
Feb 6 19:31:38 debian kernel: [ 133.976280] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 133.978310] =======================
Feb 6 19:31:38 debian kernel: [ 133.979373] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 133.995180] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 133.995192] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 133.998520] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 134.001856] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 134.008513] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 134.008523] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 134.012649] [<c0103f1b>] ? restore_nocheck+0x12/0x15
Feb 6 19:31:38 debian kernel: [ 134.015187] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 134.016607] [<c0122f03>] ? vprintk+0x2c7/0x301
Feb 6 19:31:38 debian kernel: [ 134.018046] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 134.019422] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 134.020767] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 134.021845] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 134.022657] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 134.023973] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 134.025702] [<f0836dd8>] affs_lookup+0xfb/0x241 [affs]
Feb 6 19:31:38 debian kernel: [ 134.027279] [<f0836914>] ? affs_toupper+0x0/0x11 [affs]
Feb 6 19:31:38 debian kernel: [ 134.029413] [<c0176488>] __lookup_hash+0xa1/0xd3
Feb 6 19:31:38 debian kernel: [ 134.031852] [<c01764e1>] lookup_hash+0x27/0x2a
Feb 6 19:31:38 debian kernel: [ 134.033224] [<c017847d>] sys_renameat+0xd9/0x1dd
Feb 6 19:31:38 debian kernel: [ 134.034653] [<c017ef24>] ? dput+0x93/0xde
Feb 6 19:31:38 debian kernel: [ 134.035724] [<c0170be6>] ? __fput+0x14d/0x155
Feb 6 19:31:38 debian kernel: [ 134.038511] [<c0170db7>] ? fput+0x17/0x19
Feb 6 19:31:38 debian kernel: [ 134.038511] [<c016e5f2>] ? filp_close+0x50/0x5a
Feb 6 19:31:38 debian kernel: [ 134.038511] [<c0178593>] sys_rename+0x12/0x14
Feb 6 19:31:38 debian kernel: [ 134.038511] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 134.038511] =======================
Feb 6 19:31:38 debian kernel: [ 134.038528] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 134.047851] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 134.048510] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 134.048510] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 134.048510] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 134.049745] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 134.051856] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 134.053388] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 134.054804] [<c012289e>] ? wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 134.055946] [<c0122a58>] ? release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 134.058475] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 134.058523] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 134.061843] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 134.061843] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 134.063519] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 134.068517] [<c018b87b>] ? __getblk+0x2c/0x25f
Feb 6 19:31:38 debian kernel: [ 134.069929] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 134.071184] [<f0839c18>] affs_remove_hash+0x67/0x186 [affs]
Feb 6 19:31:38 debian kernel: [ 134.071842] [<f0836c49>] affs_rename+0xb4/0x148 [affs]
Feb 6 19:31:38 debian kernel: [ 134.071842] [<c0176e10>] vfs_rename+0x18b/0x37e
Feb 6 19:31:38 debian kernel: [ 134.071842] [<c0178514>] sys_renameat+0x170/0x1dd
Feb 6 19:31:38 debian kernel: [ 134.071842] [<c0172b69>] ? sys_stat64+0x22/0x28
Feb 6 19:31:38 debian kernel: [ 134.071842] [<c0178593>] sys_rename+0x12/0x14
Feb 6 19:31:38 debian kernel: [ 134.071849] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 134.075185] =======================
Feb 6 19:31:38 debian kernel: [ 134.078508] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 134.439171] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 134.439171] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 134.442517] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 134.445837] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 134.449540] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 134.452095] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 134.452515] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 134.455844] [<c012289e>] ? wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 134.457640] [<c0122a58>] ? release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 134.459169] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 134.459169] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 134.459169] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 134.459169] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 134.460478] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 134.462000] [<c01506ba>] ? add_to_page_cache+0x78/0x80
Feb 6 19:31:38 debian kernel: [ 134.462502] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 134.462502] [<f083ad5d>] affs_symlink_readpage+0x4d/0x164 [affs]
Feb 6 19:31:38 debian kernel: [ 134.462502] [<c01506df>] ? add_to_page_cache_lru+0x1d/0x26
Feb 6 19:31:38 debian kernel: [ 134.462502] [<c015091b>] read_cache_page_async+0xa7/0x135
Feb 6 19:31:38 debian kernel: [ 134.462512] [<f083ad10>] ? affs_symlink_readpage+0x0/0x164 [affs]
Feb 6 19:31:38 debian kernel: [ 134.464406] [<c0152061>] read_cache_page+0xc/0x3f
Feb 6 19:31:38 debian kernel: [ 134.466822] [<c0175d59>] page_getlink+0x1e/0x38
Feb 6 19:31:38 debian kernel: [ 134.468485] [<c0175d8b>] page_follow_link_light+0x18/0x26
Feb 6 19:31:38 debian kernel: [ 134.469176] [<c0176611>] generic_readlink+0x28/0x6e
Feb 6 19:31:38 debian kernel: [ 134.470974] [<c01884a0>] ? __mark_inode_dirty+0x12c/0x134
Feb 6 19:31:38 debian kernel: [ 134.472502] [<c03c1dbc>] ? _spin_unlock+0x1d/0x20
Feb 6 19:31:38 debian kernel: [ 134.472509] [<c01884a0>] ? __mark_inode_dirty+0x12c/0x134
Feb 6 19:31:38 debian kernel: [ 134.474782] [<c0180e8c>] ? touch_atime+0xb7/0xbf
Feb 6 19:31:38 debian kernel: [ 134.475835] [<c01729af>] sys_readlinkat+0x59/0x73
Feb 6 19:31:38 debian kernel: [ 134.475835] [<c01729dc>] sys_readlink+0x13/0x15
Feb 6 19:31:38 debian kernel: [ 134.475835] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 134.475835] =======================
Feb 6 19:31:38 debian kernel: [ 134.475843] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 134.634856] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 134.634856] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 134.634856] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 134.634863] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 134.637744] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 134.638189] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 134.639031] [<c011a8b3>] ? __wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 134.640389] [<c012289e>] ? wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 134.641522] [<c0122a58>] ? release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 134.642490] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 134.645858] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 134.647189] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 134.648746] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 134.650297] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 134.652107] [<c018bba2>] ? __set_page_dirty+0xf4/0x103
Feb 6 19:31:38 debian kernel: [ 134.653723] [<f083ac88>] ? affs_free_block+0x107/0x18f [affs]
Feb 6 19:31:38 debian kernel: [ 134.654862] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 134.656150] [<f08395b0>] affs_truncate+0x398/0x424 [affs]
Feb 6 19:31:38 debian kernel: [ 134.657791] [<f0837216>] affs_delete_inode+0x3a/0x52 [affs]
Feb 6 19:31:38 debian kernel: [ 134.659162] [<f08371dc>] ? affs_delete_inode+0x0/0x52 [affs]
Feb 6 19:31:38 debian kernel: [ 134.661530] [<c018074e>] generic_delete_inode+0x8f/0xf9
Feb 6 19:31:38 debian kernel: [ 134.664862] [<c01807ca>] generic_drop_inode+0x12/0x129
Feb 6 19:31:38 debian kernel: [ 134.668197] [<c017fd88>] iput+0x63/0x66
Feb 6 19:31:38 debian kernel: [ 134.671520] [<c017871a>] do_unlinkat+0xb9/0x12b
Feb 6 19:31:38 debian kernel: [ 134.671520] [<c017879c>] sys_unlink+0x10/0x12
Feb 6 19:31:38 debian kernel: [ 134.672150] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 134.675660] =======================
Feb 6 19:31:38 debian kernel: [ 134.676723] ---[ end trace 00b02e0a77487d9f ]---
Feb 6 19:31:38 debian kernel: [ 134.678092] VFS: brelse: Trying to free free buffer
Feb 6 19:31:38 debian kernel: [ 134.678195] ------------[ cut here ]------------
Feb 6 19:31:38 debian kernel: [ 134.679546] WARNING: at /home/hch/work/linux-2.6/fs/buffer.c:1188 __brelse+0x28/0x2b()
Feb 6 19:31:38 debian kernel: [ 134.681525] Modules linked in: affs binfmt_misc dm_snapshot dm_mirror i2c_piix4 i2c_core
Feb 6 19:31:38 debian kernel: [ 134.684861] Pid: 2258, comm: fsstress Not tainted 2.6.24-08039-g488b5ec-dirty #36
Feb 6 19:31:38 debian kernel: [ 134.688197] [<c01222e4>] warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 134.691527] [<c0103f1b>] ? restore_nocheck+0x12/0x15
Feb 6 19:31:38 debian kernel: [ 134.693112] [<c0122a4d>] ? release_console_sem+0x1ac/0x1bf
Feb 6 19:31:38 debian kernel: [ 134.694830] [<c0122f03>] ? vprintk+0x2c7/0x301
Feb 6 19:31:38 debian kernel: [ 134.694852] [<c018b4d2>] ? __find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 134.694852] [<c0122f52>] ? printk+0x15/0x17
Feb 6 19:31:38 debian kernel: [ 134.694852] [<c018ae5a>] __brelse+0x28/0x2b
Feb 6 19:31:38 debian kernel: [ 134.694852] [<c018b83d>] __find_get_block+0x14f/0x161
Feb 6 19:31:38 debian kernel: [ 134.698193] [<c018b869>] __getblk+0x1a/0x25f
Feb 6 19:31:38 debian kernel: [ 134.699534] [<c018bba2>] ? __set_page_dirty+0xf4/0x103
Feb 6 19:31:38 debian kernel: [ 134.701518] [<f083ac88>] ? affs_free_block+0x107/0x18f [affs]
Feb 6 19:31:38 debian kernel: [ 134.701518] [<c018d3f7>] __bread+0xc/0x91
Feb 6 19:31:38 debian kernel: [ 134.701518] [<f08395b0>] affs_truncate+0x398/0x424 [affs]
Feb 6 19:31:38 debian kernel: [ 134.701533] [<f0837216>] affs_delete_inode+0x3a/0x52 [affs]
Feb 6 19:31:38 debian kernel: [ 134.704851] [<f08371dc>] ? affs_delete_inode+0x0/0x52 [affs]
Feb 6 19:31:38 debian kernel: [ 134.704851] [<c018074e>] generic_delete_inode+0x8f/0xf9
Feb 6 19:31:38 debian kernel: [ 134.704859] [<c01807ca>] generic_drop_inode+0x12/0x129
Feb 6 19:31:38 debian kernel: [ 134.708191] [<c017fd88>] iput+0x63/0x66
Feb 6 19:31:38 debian kernel: [ 134.711517] [<c017871a>] do_unlinkat+0xb9/0x12b
Feb 6 19:31:38 debian kernel: [ 134.713968] [<c017879c>] sys_unlink+0x10/0x12
Feb 6 19:31:38 debian kernel: [ 134.714863] [<c0103e32>] sysenter_past_esp+0x5f/0xa5
Feb 6 19:31:38 debian kernel: [ 134.718184] =======================Hi,
Quoted 7 lines · Christoph Hellwig
Then affs doesn't really survive fsstress from ltp very well. The first lockdep warning I got is from my implement
drop_inodepatch but even with that things look quite bad, with both lockdep warnings and lots of 'VFS: brelse: Trying to free free buffer' with following stack trace.Any idea how to proceed with affs?
With the patch below on top of yours, affs will survive a little longer. There were a few problems with recovering from allocation failures (and there are probably a few left).
bye, Roman
fs/affs/affs.h +1 -2
@@ -48,7 +48,7 @@ struct affs_ext_key {* affs fs inode data in memory */ struct affs_inode_info { -u32 i_opencnt; +atomic_t i_opencnt;struct semaphore i_link_lock;/* Protects internal inode access. */ struct semaphore i_ext_lock;/* Protects internal inode access. */ #define i_hash_lock i_ext_lock
@@ -170,7 +170,6 @@ extern int affs_rename(struct inode *oldextern unsigned long affs_parent_ino (struct inode *dir);extern struct inode *affs_new_inode (struct inode *dir);extern int affs_notify_change (struct dentry *dentry,struct iattr *attr); -extern void affs_drop_inode (struct inode *inode);extern void affs_delete_inode (struct inode *inode);extern void affs_clear_inode (struct inode *inode);extern void affs_read_inode (struct inode *inode);
fs/affs/file.c +14 -8
@@ -48,8 +48,8 @@ affs_file_open(struct inode *inode, stru {if (atomic_read (&filp->f_count) !=1 )return 0 ; -pr_debug ("AFFS: open(%d)\n" ,AFFS_I (inode)->i_opencnt); -AFFS_I (inode)->i_opencnt++; +pr_debug ("AFFS: open(%lu,%d)\n" , inode->i_ino,atomic_read (&AFFS_I (inode)->i_opencnt)); +atomic_inc (&AFFS_I (inode)->i_opencnt);return 0 ; }
@@ -58,10 +58,14 @@ affs_file_release(struct inode *inode, s {if (atomic_read (&filp->f_count) !=0 )return 0 ; -pr_debug ("AFFS: release(%d)\n" ,AFFS_I (inode)->i_opencnt); -AFFS_I (inode)->i_opencnt--; -if (!AFFS_I (inode)->i_opencnt) +pr_debug ("AFFS: release(%lu, %d)\n" , inode->i_ino,atomic_read (&AFFS_I (inode)->i_opencnt)); +if (atomic_dec_and_test (&AFFS_I (inode)->i_opencnt)) { +mutex_lock (&inode->i_mutex); +if (inode->i_size !=AFFS_I (inode)->mmu_private) +affs_truncate (inode);affs_free_prealloc (inode); +mutex_unlock (&inode->i_mutex); + }return 0 ; }
@@ -180,7 +184,7 @@ affs_get_extblock(struct inode *inode, u/* inline the simplest case: same extended block as last time */ struct buffer_head *bh =AFFS_I (inode)->i_ext_bh;if (ext ==AFFS_I (inode)->i_ext_last) -atomic_inc (&bh->b_count); +get_bh (bh);else /* we have to do more (not inlined) */ bh =affs_get_extblock_slow (inode, ext);
@@ -306,7 +310,7 @@ store_ext:affs_brelse (AFFS_I (inode)->i_ext_bh);AFFS_I (inode)->i_ext_last = ext;AFFS_I (inode)->i_ext_bh = bh; -atomic_inc (&bh->b_count); +get_bh (bh);return bh;
@@ -324,7 +328,6 @@ affs_get_block(struct inode *inode, sectpr_debug ("AFFS: get_block(%u, %lu)\n" , (u32 )inode->i_ino, (unsigned long )block); -if (block > (sector_t )0x7fffffffUL )BUG ();
@@ -834,6 +837,8 @@ affs_truncate(struct inode *inode) res = mapping->a_ops->write_begin (NULL , mapping, size,0 ,0 , &page, &fsdata);if (!res) res = mapping->a_ops->write_end (NULL , mapping, size,0 ,0 , page, fsdata); +else + inode->i_size =AFFS_I (inode)->mmu_private;mark_inode_dirty (inode);return ; }else if (inode->i_size ==AFFS_I (inode)->mmu_private)
@@ -869,6 +874,7 @@ affs_truncate(struct inode *inode) blk++; }else AFFS_HEAD (ext_bh)->first_data =0 ; +AFFS_HEAD (ext_bh)->block_count =cpu_to_be32 (i); size =AFFS_SB (sb)->s_hashsize;if (size > blkcnt - blk + i) size = blkcnt - blk + i;
fs/affs/inode.c +8 -21
@@ -53,7 +53,7 @@ affs_read_inode(struct inode *inode)AFFS_I (inode)->i_extcnt =1 ;AFFS_I (inode)->i_ext_last = ~1 ;AFFS_I (inode)->i_protect = prot; -AFFS_I (inode)->i_opencnt =0 ; +atomic_set (&AFFS_I (inode)->i_opencnt,0 );AFFS_I (inode)->i_blkcnt =0 ;AFFS_I (inode)->i_lc =NULL ;AFFS_I (inode)->i_lc_size =0 ;
@@ -103,8 +103,6 @@ affs_read_inode(struct inode *inode) inode->i_mode |=S_IFDIR ; }else inode->i_mode =S_IRUGO |S_IXUGO |S_IWUSR |S_IFDIR ; -if (tail->link_chain) - inode->i_nlink =2 ;/* Maybe it should be controlled by mount parameter? */ //inode->i_mode |= S_ISVTX; inode->i_op = &affs_dir_inode_operations;
@@ -239,26 +237,12 @@ out: }void -affs_drop_inode (struct inode *inode) -{ -affs_free_prealloc (inode); - -mutex_lock (&inode->i_mutex); -if (inode->i_size !=AFFS_I (inode)->mmu_private) -affs_truncate (inode); -mutex_unlock (&inode->i_mutex); - -generic_drop_inode (inode); -} - -void affs_delete_inode (struct inode *inode) {pr_debug ("AFFS: delete_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink);truncate_inode_pages (&inode->i_data,0 ); inode->i_size =0 ; -if (S_ISREG (inode->i_mode)) -affs_truncate (inode); +affs_truncate (inode);clear_inode (inode);affs_free_block (inode->i_sb, inode->i_ino); }
@@ -266,9 +250,12 @@ affs_delete_inode(struct inode *inode)void affs_clear_inode (struct inode *inode) { -unsigned long cache_page = (unsigned long )AFFS_I (inode)->i_lc; +unsigned long cache_page;pr_debug ("AFFS: clear_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); + +affs_free_prealloc (inode); + cache_page = (unsigned long )AFFS_I (inode)->i_lc;if (cache_page) {pr_debug ("AFFS: freeing ext cache\n" );AFFS_I (inode)->i_lc =NULL ;
@@ -305,7 +292,7 @@ affs_new_inode(struct inode *dir) inode->i_ino = block; inode->i_nlink =1 ; inode->i_mtime = inode->i_atime = inode->i_ctime =CURRENT_TIME_SEC ; -AFFS_I (inode)->i_opencnt =0 ; +atomic_set (&AFFS_I (inode)->i_opencnt,0 );AFFS_I (inode)->i_blkcnt =0 ;AFFS_I (inode)->i_lc =NULL ;AFFS_I (inode)->i_lc_size =0 ;
@@ -358,12 +345,12 @@ affs_add_entry(struct inode *dir, structswitch (type) {case ST_LINKFILE :case ST_LINKDIR : - inode_bh = bh; retval = -ENOSPC ; block =affs_alloc_block (dir, dir->i_ino);if (!block) goto err; retval = -EIO ; + inode_bh = bh; bh =affs_getzeroblk (sb, block);if (!bh) goto err;
fs/affs/namei.c +4 -2
@@ -236,7 +236,8 @@ affs_lookup(struct inode *dir, struct deint affs_unlink (struct inode *dir,struct dentry *dentry) { -pr_debug ("AFFS: unlink(dir=%d, \"%.*s\")\n" , (u32 )dir->i_ino, +pr_debug ("AFFS: unlink(dir=%d, %lu \"%.*s\")\n" , (u32 )dir->i_ino, + dentry->d_inode->i_ino, (int )dentry->d_name.len, dentry->d_name.name);return affs_remove_header (dentry);
@@ -304,7 +305,8 @@ affs_mkdir(struct inode *dir, struct denint affs_rmdir (struct inode *dir,struct dentry *dentry) { -pr_debug ("AFFS: rmdir(dir=%u, \"%.*s\")\n" , (u32 )dir->i_ino, +pr_debug ("AFFS: rmdir(dir=%u, %lu \"%.*s\")\n" , (u32 )dir->i_ino, + dentry->d_inode->i_ino, (int )dentry->d_name.len, dentry->d_name.name);return affs_remove_header (dentry);
fs/affs/super.c +11 -6
@@ -71,12 +71,18 @@ static struct kmem_cache * affs_inode_castatic struct inode *affs_alloc_inode (struct super_block *sb) { -struct affs_inode_info *ei; - ei = (struct affs_inode_info *)kmem_cache_alloc (affs_inode_cachep,GFP_KERNEL ); -if (!ei) +struct affs_inode_info *i; + + i = (struct affs_inode_info *)kmem_cache_alloc (affs_inode_cachep,GFP_KERNEL ); +if (!i)return NULL ; - ei->vfs_inode.i_version =1 ; -return &ei->vfs_inode; + + i->vfs_inode.i_version =1 ; + i->i_lc =NULL ; + i->i_ext_bh =NULL ; + i->i_pa_cnt =0 ; + +return &i->vfs_inode; }static void affs_destroy_inode (struct inode *inode)
@@ -115,7 +121,6 @@ static const struct super_operations aff .destroy_inode = affs_destroy_inode, .read_inode = affs_read_inode, .write_inode = affs_write_inode, - .drop_inode = affs_drop_inode, .delete_inode = affs_delete_inode, .clear_inode = affs_clear_inode, .put_super = affs_put_super,
Sorry for taking so long to get back to this, it had been some busy weeks..
Quoted 3 lines · Roman Zippel
With the patch below on top of yours, affs will survive a little longer. There were a few problems with recovering from allocation failures (and there are probably a few left).
Yes, with this one affs survives medium-heavy beating with fsstress and fsx for me.
Below is a version that merges in my earlier patche which it was ontop of and fixes up the minor checkpatch.pl complaints.
Any chance you could send this for inclusion into 2.6.26 so that affs works reliable there?
fs/affs/affs.h +1 -3
@@ -48,7 +48,7 @@ struct affs_ext_key {* affs fs inode data in memory */ struct affs_inode_info { -u32 i_opencnt; +atomic_t i_opencnt;struct semaphore i_link_lock;/* Protects internal inode access. */ struct semaphore i_ext_lock;/* Protects internal inode access. */ #define i_hash_lock i_ext_lock
@@ -170,8 +170,6 @@ extern int affs_rename(struct inode *oldextern unsigned long affs_parent_ino (struct inode *dir);extern struct inode *affs_new_inode (struct inode *dir);extern int affs_notify_change (struct dentry *dentry,struct iattr *attr); -extern void affs_put_inode (struct inode *inode); -extern void affs_drop_inode (struct inode *inode);extern void affs_delete_inode (struct inode *inode);extern void affs_clear_inode (struct inode *inode); extern struct inode *affs_iget(struct super_block *sb,
fs/affs/file.c +17 -8
@@ -48,8 +48,9 @@ affs_file_open(struct inode *inode, stru {if (atomic_read (&filp->f_count) !=1 )return 0 ; -pr_debug ("AFFS: open(%d)\n" ,AFFS_I (inode)->i_opencnt); -AFFS_I (inode)->i_opencnt++; +pr_debug ("AFFS: open(%lu,%d)\n" , + inode->i_ino,atomic_read (&AFFS_I (inode)->i_opencnt)); +atomic_inc (&AFFS_I (inode)->i_opencnt);return 0 ; }
@@ -58,10 +59,16 @@ affs_file_release(struct inode *inode, s {if (atomic_read (&filp->f_count) !=0 )return 0 ; -pr_debug ("AFFS: release(%d)\n" ,AFFS_I (inode)->i_opencnt); -AFFS_I (inode)->i_opencnt--; -if (!AFFS_I (inode)->i_opencnt) +pr_debug ("AFFS: release(%lu, %d)\n" , + inode->i_ino,atomic_read (&AFFS_I (inode)->i_opencnt)); + +if (atomic_dec_and_test (&AFFS_I (inode)->i_opencnt)) { +mutex_lock (&inode->i_mutex); +if (inode->i_size !=AFFS_I (inode)->mmu_private) +affs_truncate (inode);affs_free_prealloc (inode); +mutex_unlock (&inode->i_mutex); + }return 0 ; }
@@ -180,7 +187,7 @@ affs_get_extblock(struct inode *inode, u/* inline the simplest case: same extended block as last time */ struct buffer_head *bh =AFFS_I (inode)->i_ext_bh;if (ext ==AFFS_I (inode)->i_ext_last) -atomic_inc (&bh->b_count); +get_bh (bh);else /* we have to do more (not inlined) */ bh =affs_get_extblock_slow (inode, ext);
@@ -306,7 +313,7 @@ store_ext:affs_brelse (AFFS_I (inode)->i_ext_bh);AFFS_I (inode)->i_ext_last = ext;AFFS_I (inode)->i_ext_bh = bh; -atomic_inc (&bh->b_count); +get_bh (bh);return bh;
@@ -324,7 +331,6 @@ affs_get_block(struct inode *inode, sectpr_debug ("AFFS: get_block(%u, %lu)\n" , (u32 )inode->i_ino, (unsigned long )block); -if (block > (sector_t )0x7fffffffUL )BUG ();
@@ -834,6 +840,8 @@ affs_truncate(struct inode *inode) res = mapping->a_ops->write_begin (NULL , mapping, size,0 ,0 , &page, &fsdata);if (!res) res = mapping->a_ops->write_end (NULL , mapping, size,0 ,0 , page, fsdata); +else + inode->i_size =AFFS_I (inode)->mmu_private;mark_inode_dirty (inode);return ; }else if (inode->i_size ==AFFS_I (inode)->mmu_private)
@@ -869,6 +877,7 @@ affs_truncate(struct inode *inode) blk++; }else AFFS_HEAD (ext_bh)->first_data =0 ; +AFFS_HEAD (ext_bh)->block_count =cpu_to_be32 (i); size =AFFS_SB (sb)->s_hashsize;if (size > blkcnt - blk + i) size = blkcnt - blk + i;
fs/affs/inode.c +8 -26
@@ -58,7 +58,7 @@ struct inode *affs_iget(struct super_bloAFFS_I (inode)->i_extcnt =1 ;AFFS_I (inode)->i_ext_last = ~1 ;AFFS_I (inode)->i_protect = prot; -AFFS_I (inode)->i_opencnt =0 ; +atomic_set (&AFFS_I (inode)->i_opencnt,0 );AFFS_I (inode)->i_blkcnt =0 ;AFFS_I (inode)->i_lc =NULL ;AFFS_I (inode)->i_lc_size =0 ;
@@ -108,8 +108,6 @@ struct inode *affs_iget(struct super_blo inode->i_mode |=S_IFDIR ; }else inode->i_mode =S_IRUGO |S_IXUGO |S_IWUSR |S_IFDIR ; -if (tail->link_chain) - inode->i_nlink =2 ;/* Maybe it should be controlled by mount parameter? */ //inode->i_mode |= S_ISVTX; inode->i_op = &affs_dir_inode_operations;
@@ -245,31 +243,12 @@ out: }void -affs_put_inode (struct inode *inode) -{ -pr_debug ("AFFS: put_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); -affs_free_prealloc (inode); -} - -void -affs_drop_inode (struct inode *inode) -{ -mutex_lock (&inode->i_mutex); -if (inode->i_size !=AFFS_I (inode)->mmu_private) -affs_truncate (inode); -mutex_unlock (&inode->i_mutex); - -generic_drop_inode (inode); -} - -void affs_delete_inode (struct inode *inode) {pr_debug ("AFFS: delete_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink);truncate_inode_pages (&inode->i_data,0 ); inode->i_size =0 ; -if (S_ISREG (inode->i_mode)) -affs_truncate (inode); +affs_truncate (inode);clear_inode (inode);affs_free_block (inode->i_sb, inode->i_ino); }
@@ -277,9 +256,12 @@ affs_delete_inode(struct inode *inode)void affs_clear_inode (struct inode *inode) { -unsigned long cache_page = (unsigned long )AFFS_I (inode)->i_lc; +unsigned long cache_page;pr_debug ("AFFS: clear_inode(ino=%lu, nlink=%u)\n" , inode->i_ino, inode->i_nlink); + +affs_free_prealloc (inode); + cache_page = (unsigned long )AFFS_I (inode)->i_lc;if (cache_page) {pr_debug ("AFFS: freeing ext cache\n" );AFFS_I (inode)->i_lc =NULL ;
@@ -316,7 +298,7 @@ affs_new_inode(struct inode *dir) inode->i_ino = block; inode->i_nlink =1 ; inode->i_mtime = inode->i_atime = inode->i_ctime =CURRENT_TIME_SEC ; -AFFS_I (inode)->i_opencnt =0 ; +atomic_set (&AFFS_I (inode)->i_opencnt,0 );AFFS_I (inode)->i_blkcnt =0 ;AFFS_I (inode)->i_lc =NULL ;AFFS_I (inode)->i_lc_size =0 ;
@@ -369,12 +351,12 @@ affs_add_entry(struct inode *dir, structswitch (type) {case ST_LINKFILE :case ST_LINKDIR : - inode_bh = bh; retval = -ENOSPC ; block =affs_alloc_block (dir, dir->i_ino);if (!block) goto err; retval = -EIO ; + inode_bh = bh; bh =affs_getzeroblk (sb, block);if (!bh) goto err;
fs/affs/namei.c +4 -2
@@ -234,7 +234,8 @@ affs_lookup(struct inode *dir, struct deint affs_unlink (struct inode *dir,struct dentry *dentry) { -pr_debug ("AFFS: unlink(dir=%d, \"%.*s\")\n" , (u32 )dir->i_ino, +pr_debug ("AFFS: unlink(dir=%d, %lu \"%.*s\")\n" , (u32 )dir->i_ino, + dentry->d_inode->i_ino, (int )dentry->d_name.len, dentry->d_name.name);return affs_remove_header (dentry);
@@ -302,7 +303,8 @@ affs_mkdir(struct inode *dir, struct denint affs_rmdir (struct inode *dir,struct dentry *dentry) { -pr_debug ("AFFS: rmdir(dir=%u, \"%.*s\")\n" , (u32 )dir->i_ino, +pr_debug ("AFFS: rmdir(dir=%u, %lu \"%.*s\")\n" , (u32 )dir->i_ino, + dentry->d_inode->i_ino, (int )dentry->d_name.len, dentry->d_name.name);return affs_remove_header (dentry);
fs/affs/super.c +11 -7
@@ -71,12 +71,18 @@ static struct kmem_cache * affs_inode_castatic struct inode *affs_alloc_inode (struct super_block *sb) { -struct affs_inode_info *ei; - ei = (struct affs_inode_info *)kmem_cache_alloc (affs_inode_cachep,GFP_KERNEL ); -if (!ei) +struct affs_inode_info *i; + + i =kmem_cache_alloc (affs_inode_cachep,GFP_KERNEL ); +if (!i)return NULL ; - ei->vfs_inode.i_version =1 ; -return &ei->vfs_inode; + + i->vfs_inode.i_version =1 ; + i->i_lc =NULL ; + i->i_ext_bh =NULL ; + i->i_pa_cnt =0 ; + +return &i->vfs_inode; }static void affs_destroy_inode (struct inode *inode)
@@ -114,8 +120,6 @@ static const struct super_operations aff .alloc_inode = affs_alloc_inode, .destroy_inode = affs_destroy_inode, .write_inode = affs_write_inode, - .put_inode = affs_put_inode, - .drop_inode = affs_drop_inode, .delete_inode = affs_delete_inode, .clear_inode = affs_clear_inode, .put_super = affs_put_super,
PATCH 1/1 kill ->put_inode
And with that last patch to affs killing the last put_inode instance we can finally, after many years of transition kill this racy and awkward interface.
(It's kinda funny that even the description in Documentation/filesystems/vfs.txt was entirely wrong..)
Also remove a very misleading comment above the defintion of struct super_operations.
Signed-off-by Christoph Hellwig <[email protected]>
Documentation/filesystems/Locking +0 -2
@@ -92,7 +92,6 @@ prototypes: void (*destroy_inode)(struct inode *); void (*dirty_inode) (struct inode *); int (*write_inode) (struct inode *, int); - void (*put_inode) (struct inode *); void (*drop_inode) (struct inode *); void (*delete_inode) (struct inode *); void (*put_super) (struct super_block *);
@@ -115,7 +114,6 @@ alloc_inode: no no no destroy_inode: no dirty_inode: no (must not sleep) write_inode: no -put_inode: no drop_inode: no !!!inode_lock!!! delete_inode: no put_super: yes yes no
Documentation/filesystems/vfs.txt +0 -4
@@ -205,7 +205,6 @@ struct super_operations { void (*dirty_inode) (struct inode *); int (*write_inode) (struct inode *, int); - void (*put_inode) (struct inode *); void (*drop_inode) (struct inode *); void (*delete_inode) (struct inode *); void (*put_super) (struct super_block *);
@@ -246,9 +245,6 @@ or bottom half). inode to disc. The second parameter indicates whether the write should be synchronous or not, not all filesystems check this flag. - put_inode: called when the VFS inode is removed from the inode - cache. - drop_inode: called when the last access to the inode is dropped, with the inode_lock spinlock held.
fs/inode.c +0 -3
@@ -1155,9 +1155,6 @@ void iput(struct inode *inode)BUG_ON (inode->i_state ==I_CLEAR ); -if (op && op->put_inode) - op->put_inode (inode); -if (atomic_dec_and_lock (&inode->i_count, &inode_lock))iput_final (inode); }
include/linux/fs.h +0 -5
@@ -1289,17 +1289,12 @@ extern ssize_t vfs_readv(struct file *,extern ssize_t vfs_writev (struct file *,const struct iovec __user *,unsigned long ,loff_t *); -/* -* NOTE: write_inode, delete_inode, clear_inode, put_inode can be called -* without the big kernel lock held in all filesystems. -*/ struct super_operations {struct inode *(*alloc_inode)(struct super_block *sb);void (*destroy_inode)(struct inode *);void (*dirty_inode) (struct inode *);int (*write_inode) (struct inode *,int ); -void (*put_inode) (struct inode *);void (*drop_inode) (struct inode *);void (*delete_inode) (struct inode *);void (*put_super) (struct super_block *);
Quoted 8 lines · On Tue, 29 Apr 2008 17:02:20 +0200 Christoph Hellwig <[email protected]> wrote:
Sorry for taking so long to get back to this, it had been some busy weeks..
Quoted 3 lines · Roman Zippel
With the patch below on top of yours, affs will survive a little longer. There were a few problems with recovering from allocation failures (and there are probably a few left).
Quoted 8 lines
Yes, with this one affs survives medium-heavy beating with fsstress and fsx for me.
Below is a version that merges in my earlier patche which it was ontop of and fixes up the minor checkpatch.pl complaints.
Any chance you could send this for inclusion into 2.6.26 so that affs works reliable there?
Could we get the authorship, changelog and signoffs sorted out please?
Hi,
Quoted 1 line · Andrew Morton
Could we get the authorship, changelog and signoffs sorted out please?
Sorry for the delay, here it is:
- remove affs_put_inode, so preallocations aren't discared unnecessarily often.
- remove affs_drop_inode, it's called with a spinlock held, so it can't use a mutex.
- make i_opencnt atomic
- avoid direct b_count manipulations
- a few allocation failure fixes, so that these are more gracefully handled now.
Signed-off-by Roman Zippel <[email protected]>
Signed-off-by Christoph Hellwig <[email protected]>