* overlayfs: BUG in ovl_whiteout
@ 2010-10-01 18:07 Andy Whitcroft
2010-10-04 9:23 ` Miklos Szeredi
0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-01 18:07 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, vaurora, neilb, viro
I have been playing with overlayfs for a bit now and there seems to
be something a little dodgy when unmounting and remounting an overlay.
I modified some files in the overlay, immediatly unmounted the overlay and
again immediatly remounted it. On touching the same files in the overlay
I triggered the following BUG (dmesg fragment at the bottom of this email).
When trying to reproduce this I also triggered a hard hang.
This is the specific BUG in my source:
static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
{
[...]
/* Just been removed within the same locked region */
BUG_ON(newdentry->d_inode);
[...]
}
All of this is against the V3 patches.
-apw
[ 3828.863867] ------------[ cut here ]------------
[ 3828.863898] kernel BUG at /home/apw/build/maverick/ubuntu-natty/fs/overlayfs/overlayfs.c:1441!
[ 3828.863940] invalid opcode: 0000 [#1] SMP
[ 3828.863965] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.6/local_cpus
[ 3828.863998] CPU 1
[ 3828.864010] Modules linked in: overlayfs binfmt_misc ppdev kvm_intel kvm i915 drm_kms_helper snd_hda_codec_idt drm snd_hda_intel i2c_algo_bit snd_hda_codec video snd_hwdep tpm_tis tpm coretemp snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event output snd_seq tpm_bios intel_agp snd_timer lp snd_seq_device parport snd soundcore snd_page_alloc hid_belkin usbhid hid ahci e1000e libahci
[ 3828.864254]
[ 3828.864266] Pid: 18395, comm: apt-get Not tainted 2.6.36-0-generic #1~overlayfs201010011723 BB Name To be filled by O.E.M./Product Name To Be Filled By O.E.M.
[ 3828.864330] RIP: 0010:[<ffffffffa02ca173>] [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
[ 3828.864379] RSP: 0018:ffff8800670e1c28 EFLAGS: 00010282
[ 3828.864405] RAX: ffff8800263f3600 RBX: ffff88006711ec00 RCX: 0000000000000000
[ 3828.864439] RDX: ffff8800263f3600 RSI: ffff88006aefe4c2 RDI: ffff8800263f3608
[ 3828.864472] RBP: ffff8800670e1c58 R08: 0000000000000650 R09: ffff8800765d7e80
[ 3828.864506] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800263f3600
[ 3828.864539] R13: ffff88006e097240 R14: ffff880063881780 R15: 00000000fffffff4
[ 3828.864573] FS: 0000000000000000(0000) GS:ffff880001e80000(0063) knlGS:00000000f73d26d0
[ 3828.864612] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 3828.864639] CR2: 00000000f770e4c0 CR3: 000000007808d000 CR4: 00000000000406e0
[ 3828.864673] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 3828.864706] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 3828.864740] Process apt-get (pid: 18395, threadinfo ffff8800670e0000, task ffff88006f4f2dc0)
[ 3828.864779] Stack:
[ 3828.864790] ffff8800670e1c58 ffff8800263bba80 ffff8800263bba80 0000000000000000
[ 3828.864832] <0> ffff8800263f3600 ffff880063881780 ffff8800670e1cf8 ffffffffa02cbbe9
[ 3828.864877] <0> ffff8800670e1d08 0000000000000001 ffff880063881701 ffff8800263f3600
[ 3828.864924] Call Trace:
[ 3828.864940] [<ffffffffa02cbbe9>] ovl_rename+0x319/0x3d0 [overlayfs]
[ 3828.864976] [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
[ 3828.865009] [<ffffffff81163db9>] ? dentry_permission+0x99/0xc0
[ 3828.865038] [<ffffffff81163a4b>] vfs_rename_other+0xcb/0x130
[ 3828.865067] [<ffffffff81164fdb>] vfs_rename+0x14b/0x240
[ 3828.865094] [<ffffffff81163555>] ? __lookup_hash+0x55/0xe0
[ 3828.865121] [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
[ 3828.865154] [<ffffffff81166d04>] sys_renameat+0x254/0x280
[ 3828.865184] [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
[ 3828.865212] [<ffffffff812c3bcd>] ? _atomic_dec_and_lock+0x4d/0x80
[ 3828.865243] [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
[ 3828.865271] [<ffffffff81157993>] ? sys_fchmodat+0x73/0x110
[ 3828.865300] [<ffffffff81048173>] ? sys32_stat64+0x33/0x40
[ 3828.865328] [<ffffffff81166d4b>] sys_rename+0x1b/0x20
[ 3828.865354] [<ffffffff81047230>] sysenter_dispatch+0x7/0x2e
[ 3828.865382] Code: eb d4 eb 04 90 90 90 90 48 8b 35 a1 2a 00 00 45 31 c0 b9 01 00 00 00 48 c7 c2 25 ce 2c a0 4c 89 e7 e8 92 fa ea e0 41 89 c7 eb 91 <0f> 0b eb fe 41 89 c7 eb 90 eb 02 90 90 55 48 89 e5 48 83 ec 20
[ 3828.865610] RIP [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
[ 3828.866873] RSP <ffff8800670e1c28>
[ 3828.889817] ---[ end trace ae40f79401a99f83 ]---
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: overlayfs: BUG in ovl_whiteout
2010-10-01 18:07 overlayfs: BUG in ovl_whiteout Andy Whitcroft
@ 2010-10-04 9:23 ` Miklos Szeredi
2010-10-04 17:33 ` Andy Whitcroft
0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-04 9:23 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel
On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> I have been playing with overlayfs for a bit now and there seems to
> be something a little dodgy when unmounting and remounting an overlay.
> I modified some files in the overlay, immediatly unmounted the overlay and
> again immediatly remounted it. On touching the same files in the overlay
> I triggered the following BUG (dmesg fragment at the bottom of this email).
> When trying to reproduce this I also triggered a hard hang.
Thanks for the bug report, Andy.
Please change that BUG_ON to a WARN_ON. This will make this a
harmless error message in the log instead of a hang.
>
> This is the specific BUG in my source:
>
> static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
> {
> [...]
> /* Just been removed within the same locked region */
> BUG_ON(newdentry->d_inode);
> [...]
> }
>
> All of this is against the V3 patches.
What are the filesystems used as the upper and lower layers of the
overlay?
Thanks,
Miklos
>
> -apw
>
> [ 3828.863867] ------------[ cut here ]------------
> [ 3828.863898] kernel BUG at /home/apw/build/maverick/ubuntu-natty/fs/overlayfs/overlayfs.c:1441!
> [ 3828.863940] invalid opcode: 0000 [#1] SMP
> [ 3828.863965] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.6/local_cpus
> [ 3828.863998] CPU 1
> [ 3828.864010] Modules linked in: overlayfs binfmt_misc ppdev kvm_intel kvm i915 drm_kms_helper snd_hda_codec_idt drm snd_hda_intel i2c_algo_bit snd_hda_codec video snd_hwdep tpm_tis tpm coretemp snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event output snd_seq tpm_bios intel_agp snd_timer lp snd_seq_device parport snd soundcore snd_page_alloc hid_belkin usbhid hid ahci e1000e libahci
> [ 3828.864254]
> [ 3828.864266] Pid: 18395, comm: apt-get Not tainted 2.6.36-0-generic #1~overlayfs201010011723 BB Name To be filled by O.E.M./Product Name To Be Filled By O.E.M.
> [ 3828.864330] RIP: 0010:[<ffffffffa02ca173>] [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
> [ 3828.864379] RSP: 0018:ffff8800670e1c28 EFLAGS: 00010282
> [ 3828.864405] RAX: ffff8800263f3600 RBX: ffff88006711ec00 RCX: 0000000000000000
> [ 3828.864439] RDX: ffff8800263f3600 RSI: ffff88006aefe4c2 RDI: ffff8800263f3608
> [ 3828.864472] RBP: ffff8800670e1c58 R08: 0000000000000650 R09: ffff8800765d7e80
> [ 3828.864506] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800263f3600
> [ 3828.864539] R13: ffff88006e097240 R14: ffff880063881780 R15: 00000000fffffff4
> [ 3828.864573] FS: 0000000000000000(0000) GS:ffff880001e80000(0063) knlGS:00000000f73d26d0
> [ 3828.864612] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
> [ 3828.864639] CR2: 00000000f770e4c0 CR3: 000000007808d000 CR4: 00000000000406e0
> [ 3828.864673] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 3828.864706] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 3828.864740] Process apt-get (pid: 18395, threadinfo ffff8800670e0000, task ffff88006f4f2dc0)
> [ 3828.864779] Stack:
> [ 3828.864790] ffff8800670e1c58 ffff8800263bba80 ffff8800263bba80 0000000000000000
> [ 3828.864832] <0> ffff8800263f3600 ffff880063881780 ffff8800670e1cf8 ffffffffa02cbbe9
> [ 3828.864877] <0> ffff8800670e1d08 0000000000000001 ffff880063881701 ffff8800263f3600
> [ 3828.864924] Call Trace:
> [ 3828.864940] [<ffffffffa02cbbe9>] ovl_rename+0x319/0x3d0 [overlayfs]
> [ 3828.864976] [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
> [ 3828.865009] [<ffffffff81163db9>] ? dentry_permission+0x99/0xc0
> [ 3828.865038] [<ffffffff81163a4b>] vfs_rename_other+0xcb/0x130
> [ 3828.865067] [<ffffffff81164fdb>] vfs_rename+0x14b/0x240
> [ 3828.865094] [<ffffffff81163555>] ? __lookup_hash+0x55/0xe0
> [ 3828.865121] [<ffffffff8126af5f>] ? security_inode_permission+0x1f/0x30
> [ 3828.865154] [<ffffffff81166d04>] sys_renameat+0x254/0x280
> [ 3828.865184] [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
> [ 3828.865212] [<ffffffff812c3bcd>] ? _atomic_dec_and_lock+0x4d/0x80
> [ 3828.865243] [<ffffffff8159615e>] ? _raw_spin_lock+0xe/0x20
> [ 3828.865271] [<ffffffff81157993>] ? sys_fchmodat+0x73/0x110
> [ 3828.865300] [<ffffffff81048173>] ? sys32_stat64+0x33/0x40
> [ 3828.865328] [<ffffffff81166d4b>] sys_rename+0x1b/0x20
> [ 3828.865354] [<ffffffff81047230>] sysenter_dispatch+0x7/0x2e
> [ 3828.865382] Code: eb d4 eb 04 90 90 90 90 48 8b 35 a1 2a 00 00 45 31 c0 b9 01 00 00 00 48 c7 c2 25 ce 2c a0 4c 89 e7 e8 92 fa ea e0 41 89 c7 eb 91 <0f> 0b eb fe 41 89 c7 eb 90 eb 02 90 90 55 48 89 e5 48 83 ec 20
> [ 3828.865610] RIP [<ffffffffa02ca173>] ovl_whiteout+0x113/0x120 [overlayfs]
> [ 3828.866873] RSP <ffff8800670e1c28>
> [ 3828.889817] ---[ end trace ae40f79401a99f83 ]---
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: overlayfs: BUG in ovl_whiteout
2010-10-04 9:23 ` Miklos Szeredi
@ 2010-10-04 17:33 ` Andy Whitcroft
2010-10-04 20:16 ` Miklos Szeredi
0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-04 17:33 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel
On Mon, Oct 04, 2010 at 11:23:16AM +0200, Miklos Szeredi wrote:
> On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> > I have been playing with overlayfs for a bit now and there seems to
> > be something a little dodgy when unmounting and remounting an overlay.
> > I modified some files in the overlay, immediatly unmounted the overlay and
> > again immediatly remounted it. On touching the same files in the overlay
> > I triggered the following BUG (dmesg fragment at the bottom of this email).
> > When trying to reproduce this I also triggered a hard hang.
>
> Thanks for the bug report, Andy.
>
> Please change that BUG_ON to a WARN_ON. This will make this a
> harmless error message in the log instead of a hang.
Applied your patch to move these to WARN_ON. Now I get a WARN_ON after
the remount. Now that we don't explode I get the following error from
the triggering command. I assume that this is the rename which has
triggered the issue:
W: Failed to fetch http://192.168.0.60:3142/archive.ubuntu.com/ubuntu/dists/maverick/Release rename failed, File exists (/var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release -> /var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release).
I note that the file does indeed exist as a real file in both layers. I
have not as yet managed to generate a naive test that also triggers
this.
-apw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: overlayfs: BUG in ovl_whiteout
2010-10-04 17:33 ` Andy Whitcroft
@ 2010-10-04 20:16 ` Miklos Szeredi
2010-10-06 12:29 ` Andy Whitcroft
0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-04 20:16 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel
On Mon, 4 Oct 2010, Andy Whitcroft wrote:
> On Mon, Oct 04, 2010 at 11:23:16AM +0200, Miklos Szeredi wrote:
> > On Fri, 1 Oct 2010, Andy Whitcroft wrote:
> > > I have been playing with overlayfs for a bit now and there seems to
> > > be something a little dodgy when unmounting and remounting an overlay.
> > > I modified some files in the overlay, immediatly unmounted the overlay and
> > > again immediatly remounted it. On touching the same files in the overlay
> > > I triggered the following BUG (dmesg fragment at the bottom of this email).
> > > When trying to reproduce this I also triggered a hard hang.
> >
> > Thanks for the bug report, Andy.
> >
> > Please change that BUG_ON to a WARN_ON. This will make this a
> > harmless error message in the log instead of a hang.
>
> Applied your patch to move these to WARN_ON. Now I get a WARN_ON after
> the remount. Now that we don't explode I get the following error from
> the triggering command. I assume that this is the rename which has
> triggered the issue:
>
> W: Failed to fetch http://192.168.0.60:3142/archive.ubuntu.com/ubuntu/dists/maverick/Release rename failed, File exists (/var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release -> /var/lib/apt/lists/192.168.0.60:3142_archive.ubuntu.com_ubuntu_dists_maverick_Release).
>
> I note that the file does indeed exist as a real file in both layers. I
> have not as yet managed to generate a naive test that also triggers
> this.
OK, my guess is that it's a 'rename to self' which was not properly
implemented.
Does the following patch make a difference?
Thanks,
Miklos
---
fs/namei.c | 19 ++++++++++++++-----
fs/overlayfs/overlayfs.c | 19 ++++++++++++++++++-
include/linux/fs.h | 3 ++-
3 files changed, 34 insertions(+), 7 deletions(-)
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c 2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/fs/namei.c 2010-10-04 21:46:13.000000000 +0200
@@ -2620,6 +2620,17 @@ static int vfs_rename_other(struct inode
return error;
}
+bool vfs_is_same_inode(struct dentry *d1, struct dentry *d2)
+{
+ BUG_ON(d1->d_sb != d2->d_sb);
+
+ if (d1->d_sb->s_op->is_same_inode)
+ return d1->d_sb->s_op->is_same_inode(d1, d2);
+ else
+ return d1->d_inode == d2->d_inode;
+}
+EXPORT_SYMBOL(vfs_is_same_inode);
+
int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry)
{
@@ -2627,11 +2638,9 @@ int vfs_rename(struct inode *old_dir, st
int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
const unsigned char *old_name;
- if (old_dentry->d_inode == new_dentry->d_inode &&
- !(old_dir->i_sb->s_type->fs_flags & FS_RENAME_SELF_ALLOW)) {
- return 0;
- }
-
+ if (vfs_is_same_inode(old_dentry, new_dentry))
+ return 0;
+
error = may_delete(old_dir, old_dentry, is_dir);
if (error)
return error;
Index: linux-2.6/fs/overlayfs/overlayfs.c
===================================================================
--- linux-2.6.orig/fs/overlayfs/overlayfs.c 2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/fs/overlayfs/overlayfs.c 2010-10-04 21:00:05.000000000 +0200
@@ -1962,10 +1962,28 @@ static int ovl_statfs(struct dentry *den
return path.dentry->d_sb->s_op->statfs(path.dentry, buf);
}
+static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
+{
+ struct dentry *upperd1;
+ struct dentry *upperd2;
+
+ upperd1 = ovl_dentry_upper(d1);
+ upperd2 = ovl_dentry_upper(d2);
+
+ if (upperd1 && upperd2)
+ return vfs_is_same_inode(upperd1, upperd2);
+
+ if (upperd1 || upperd2)
+ return false;
+
+ return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
+}
+
static const struct super_operations ovl_super_operations = {
.put_super = ovl_put_super,
.remount_fs = ovl_remount_fs,
.statfs = ovl_statfs,
+ .is_same_inode = ovl_is_same_inode,
};
struct ovl_config {
@@ -2155,7 +2173,6 @@ static int ovl_get_sb(struct file_system
static struct file_system_type ovl_fs_type = {
.owner = THIS_MODULE,
.name = "overlayfs",
- .fs_flags = FS_RENAME_SELF_ALLOW,
.get_sb = ovl_get_sb,
.kill_sb = kill_anon_super,
};
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h 2010-10-04 19:40:20.000000000 +0200
+++ linux-2.6/include/linux/fs.h 2010-10-04 21:47:40.000000000 +0200
@@ -179,7 +179,6 @@ struct inodes_stat_t {
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move()
* during rename() internally.
*/
-#define FS_RENAME_SELF_ALLOW 65536 /* Allow rename to same inode */
/*
* These are the fs-independent mount-flags: up to 32 flags are supported
@@ -1579,6 +1578,7 @@ struct super_operations {
ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
#endif
int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
+ bool (*is_same_inode)(struct dentry *, struct dentry *);
};
/*
@@ -1816,6 +1816,7 @@ extern int vfs_statfs(struct path *, str
extern int statfs_by_dentry(struct dentry *, struct kstatfs *);
extern int freeze_super(struct super_block *super);
extern int thaw_super(struct super_block *super);
+extern bool vfs_is_same_inode(struct dentry *d1, struct dentry *d2);
extern int current_umask(void);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: overlayfs: BUG in ovl_whiteout
2010-10-04 20:16 ` Miklos Szeredi
@ 2010-10-06 12:29 ` Andy Whitcroft
2010-10-06 16:51 ` Miklos Szeredi
0 siblings, 1 reply; 6+ messages in thread
From: Andy Whitcroft @ 2010-10-06 12:29 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel
On Mon, Oct 04, 2010 at 10:16:29PM +0200, Miklos Szeredi wrote:
>
> OK, my guess is that it's a 'rename to self' which was not properly
> implemented.
>
> Does the following patch make a difference?
That one panic'd immediatly. Seems that we can have missmatched lowers
as well. I applied the below over the top, and that seems to pass
testing without tripping the WARN_ON.
-apw
commit 03375858c62f617846b5dc4968fe8178eda51a7c
Author: Andy Whitcroft <apw@canonical.com>
Date: Tue Oct 5 14:54:08 2010 +0100
overlayfs: handle missing lower inodes in ovl_is_same_inode
Signed-off-by: Andy Whitcroft <apw@canonical.com>
diff --git a/fs/overlayfs/overlayfs.c b/fs/overlayfs/overlayfs.c
index c10cc7b..f596222 100644
--- a/fs/overlayfs/overlayfs.c
+++ b/fs/overlayfs/overlayfs.c
@@ -1964,19 +1964,25 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
{
- struct dentry *upperd1;
- struct dentry *upperd2;
+ struct dentry *od1;
+ struct dentry *od2;
- upperd1 = ovl_dentry_upper(d1);
- upperd2 = ovl_dentry_upper(d2);
+ od1 = ovl_dentry_upper(d1);
+ od2 = ovl_dentry_upper(d2);
- if (upperd1 && upperd2)
- return vfs_is_same_inode(upperd1, upperd2);
+ if (od1 && od2)
+ return vfs_is_same_inode(od1, od2);
- if (upperd1 || upperd2)
+ if (od1 || od2)
return false;
- return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
+ od1 = ovl_dentry_lower(d1);
+ od2 = ovl_dentry_lower(d2);
+
+ if (od1 && od2)
+ return vfs_is_same_inode(od1, od2);
+
+ return false;
}
static const struct super_operations ovl_super_operations = {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: overlayfs: BUG in ovl_whiteout
2010-10-06 12:29 ` Andy Whitcroft
@ 2010-10-06 16:51 ` Miklos Szeredi
0 siblings, 0 replies; 6+ messages in thread
From: Miklos Szeredi @ 2010-10-06 16:51 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: miklos, linux-fsdevel, linux-kernel
On Wed, 6 Oct 2010, Andy Whitcroft wrote:
> On Mon, Oct 04, 2010 at 10:16:29PM +0200, Miklos Szeredi wrote:
> >
> > OK, my guess is that it's a 'rename to self' which was not properly
> > implemented.
> >
> > Does the following patch make a difference?
>
> That one panic'd immediatly. Seems that we can have missmatched lowers
> as well. I applied the below over the top, and that seems to pass
> testing without tripping the WARN_ON.
>
> -apw
>
> commit 03375858c62f617846b5dc4968fe8178eda51a7c
> Author: Andy Whitcroft <apw@canonical.com>
> Date: Tue Oct 5 14:54:08 2010 +0100
>
> overlayfs: handle missing lower inodes in ovl_is_same_inode
>
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
OK, that's because d1 or d2 is negative, otherwise either the upper or
the lower dentry _must_ exist. In the version I committed to the
overlayfs.v3 branch vfs_is_same_inode() checks whether the dentries
are positive and only calls ->is_same_inode() if they are.
Can you please try pull from that branch and check if it's still buggy
without your patch?
Thanks,
Miklos
>
> diff --git a/fs/overlayfs/overlayfs.c b/fs/overlayfs/overlayfs.c
> index c10cc7b..f596222 100644
> --- a/fs/overlayfs/overlayfs.c
> +++ b/fs/overlayfs/overlayfs.c
> @@ -1964,19 +1964,25 @@ static int ovl_statfs(struct dentry *dentry, struct kstatfs *buf)
>
> static bool ovl_is_same_inode(struct dentry *d1, struct dentry *d2)
> {
> - struct dentry *upperd1;
> - struct dentry *upperd2;
> + struct dentry *od1;
> + struct dentry *od2;
>
> - upperd1 = ovl_dentry_upper(d1);
> - upperd2 = ovl_dentry_upper(d2);
> + od1 = ovl_dentry_upper(d1);
> + od2 = ovl_dentry_upper(d2);
>
> - if (upperd1 && upperd2)
> - return vfs_is_same_inode(upperd1, upperd2);
> + if (od1 && od2)
> + return vfs_is_same_inode(od1, od2);
>
> - if (upperd1 || upperd2)
> + if (od1 || od2)
> return false;
>
> - return vfs_is_same_inode(ovl_dentry_lower(d1), ovl_dentry_lower(d2));
> + od1 = ovl_dentry_lower(d1);
> + od2 = ovl_dentry_lower(d2);
> +
> + if (od1 && od2)
> + return vfs_is_same_inode(od1, od2);
> +
> + return false;
> }
>
> static const struct super_operations ovl_super_operations = {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-06 16:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-01 18:07 overlayfs: BUG in ovl_whiteout Andy Whitcroft
2010-10-04 9:23 ` Miklos Szeredi
2010-10-04 17:33 ` Andy Whitcroft
2010-10-04 20:16 ` Miklos Szeredi
2010-10-06 12:29 ` Andy Whitcroft
2010-10-06 16:51 ` Miklos Szeredi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).