Re: [PATCH][RFC] fix reservation discarding in affs
Hi,
On Thu, 7 Feb 2008, Christoph Hellwig wrote:
> 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?
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
Index: linux/fs/affs/affs.h
===
--- linux.orig/fs/affs/affs.h
+++ linux/fs/affs/affs.h
@@ -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 *old
extern unsigned longaffs_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);
Index: linux/fs/affs/file.c
===
--- linux.orig/fs/affs/file.c
+++ linux/fs/affs/file.c
@@ -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, sect
pr_debug("AFFS: get_block(%u, %lu)\n", (u32)inode->i_ino, (unsigned
long)block);
-
if (block > (sector_t)0x7fffUL)
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;
Index: linux/fs/affs/inode.c
===
--- linux.orig/fs/affs/inode.c
+++ linux/fs/affs/inode.c
@@ -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(inod
Re: [PATCH][RFC] fix reservation discarding in affs
On Mon, Jan 14, 2008 at 04:53:53AM +0100, Roman Zippel wrote:
> Hi,
>
> On Thu, 10 Jan 2008, Christoph Hellwig wrote:
>
> > 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: [] 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: [] 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: [] do_rmdir+0x6d/0xc4
Feb 6 19:31:01 debian kernel: [ 96.794749] #1:
(&sb->s_type->i_mutex_key#10){--..}, at: [] 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] []
__lock_acquire+0x7b4/0xab3
Feb 6 19:31:01 debian kernel: [ 96.794749] []
lock_acquire+0x57/0x73
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ?
affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] []
mutex_lock_nested+0xc6/0x21d
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ?
affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ?
affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] []
affs_put_inode+0x21/0x43 [affs]
Feb 6 19:31:01 debian kernel: [ 96.794749] [] iput+0x2f/0x66
Feb 6 19:31:01 debian kernel: [ 96.794749] []
dentry_iput+0x7c/0x97
Feb 6 19:31:01 debian kernel: [ 96.794749] [] d_kill+0x1c/0x36
Feb 6 19:31:01 debian kernel: [ 96.794749] []
prune_one_dentry+0x8d/0xa0
Feb 6 19:31:01 debian kernel: [ 96.794749] []
prune_dcache+0xcd/0x138
Feb 6 19:31:01 debian kernel: [ 96.794749] []
shrink_dcache_parent+0x1c/0xe4
Feb 6 19:31:01 debian kernel: [ 96.794749] []
dentry_unhash+0x1e/0x72
Feb 6 19:31:01 debian kernel: [ 96.794749] [] vfs_rmdir+0x7b/0xd4
Feb 6 19:31:01 debian kernel: [ 96.794749] [] do_rmdir+0x8d/0xc4
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ? fput+0x17/0x19
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ?
sysenter_past_esp+0x9a/0xa5
Feb 6 19:31:01 debian kernel: [ 96.794749] [] ?
trace_hardirqs_on+0xe1/0x102
Feb 6 19:31:01 debian kernel: [ 96.794749] [] sys_rmdir+0x10/0x12
Feb 6 19:31:01 debian kernel: [ 96.794749] []
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] []
warn_on_slowpath+0x41/0x51
Feb 6 19:31:38 debian kernel: [ 133.898316] [] ?
__wake_up+0x31/0x3b
Feb 6 19:31:38 debian kernel: [ 133.899925] [] ?
wake_up_klogd+0x2e/0x31
Feb 6 19:31:38 debian kernel: [ 133.902495] [] ?
release_console_sem+0x1b7/0x1bf
Feb 6 19:31:38 debian kernel: [ 133.904973] [] ?
__find_get_block_slow+0xf2/0x105
Feb 6 19:31:38 debian kernel: [ 133.906135] [] ? printk+0x15
Re: [PATCH][RFC] fix reservation discarding in affs
Hi, On Thu, 10 Jan 2008, Christoph Hellwig wrote: > 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 - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC] fix reservation discarding in affs
On Thu, Feb 10, 2005 at 01:57:04PM +0100, Roman Zippel wrote:
> > affs already does it in a) and a few other places, so adding it to
> > affs_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.
>
> 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]>
Index: linux-2.6/fs/affs/affs.h
===
--- linux-2.6.orig/fs/affs/affs.h 2008-01-10 10:46:07.0 +0100
+++ linux-2.6/fs/affs/affs.h2008-01-10 10:46:08.0 +0100
@@ -170,7 +170,6 @@ extern int affs_rename(struct inode *old
extern unsigned longaffs_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);
Index: linux-2.6/fs/affs/inode.c
===
--- linux-2.6.orig/fs/affs/inode.c 2008-01-10 10:46:07.0 +0100
+++ linux-2.6/fs/affs/inode.c 2008-01-10 16:06:25.0 +0100
@@ -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);
Index: linux-2.6/fs/affs/super.c
===
--- linux-2.6.orig/fs/affs/super.c 2008-01-10 10:46:07.0 +0100
+++ linux-2.6/fs/affs/super.c 2008-01-10 10:46:08.0 +0100
@@ -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,
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC] fix reservation discarding in affs
Hi, On Thu, 10 Feb 2005, Christoph Hellwig wrote: > > affs already does it in a) and a few other places, so adding it to > > affs_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. > > 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 - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC] fix reservation discarding in affs
On Sun, Dec 12, 2004 at 02:45:45PM +0100, Christoph Hellwig wrote:
> 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 to
> affs_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.
ping?
--- 1.21/fs/affs/inode.c2004-09-17 08:58:42 +02:00
+++ edited/fs/affs/inode.c 2004-12-10 20:21:40 +01:00
@@ -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 =
--- 1.46/fs/affs/super.c2004-09-04 05:11:01 +02:00
+++ edited/fs/affs/super.c 2004-12-10 20:19:18 +01:00
@@ -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,
--- 1.9/include/linux/affs_fs.h 2004-09-17 08:58:43 +02:00
+++ edited/include/linux/affs_fs.h 2004-12-10 20:19:12 +01:00
@@ -58,7 +58,6 @@
extern unsigned longaffs_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);
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
