On May 20, 2011, at 10:17 AM, Miklos Szeredi wrote:
> Miklos Szeredi <[email protected]> writes:
>
>> Erez Zadok <[email protected]> writes:
>>
>>> I tried your overlayfs.v9 git repo w/ racer, using two separate ext3
>>> filesystems (one for lowerdir and another for upperdir). I got the
>>> WARN_ON in ovl_permission to trigger within about 10 minutes of
>>> testing. Looking at the code, I see a problem in returning w/o
>>> cleaning up an dput-ing the alias dentry. Simple patch enclosed
>>> below.
>>
>> Hmm, thanks. The more interesting question is: why does that WARN_ON
>> trigger? I'll look into it.
>
> I think I found the cause of all the bug and oopsen you are seeing.
>
> Overlayfs expects upper and lower dentries to be always positive, it
> never stores negative dentries there, there's no point, instead it
> stores NULL.
>
> There are basically two ways a positive dentry can become negative:
>
> A) dentry becomes unhashed with d_count == 0
>
> B) d_delete with d_count == 1
>
> Case A is not possible in our case, since overlayfs keeps a ref on the
> upper/lower dentries for the lifetime of the overlayfs dentry.
>
> Case B is however possible, since no extra ref is taken before calling
> vfs_unlink/vfs_rmdir. So it looks like this is being triggered.
>
> This is easy to solve, just grab a ref to upperdentry before
> unlink/rmdir. Equivalent is if we grab an extra reference from the
> start. The below patch does this.
>
> With the patch I can't trigger the bugs anymore.
>
> Erez, could you please also check if reverting your patches and applying
> this one fixes all the bugs?
Done: reverted my ovl patches and applied yours. And indeed, the NULL oopses don't show up after 10 minutes of running. Just to be sure, I ran overlayfs on top of ext2, ext3, xfs, reiserfs, and tmps ? no oopses. And I wasn't able to reproduce the WARN_ON in ovl_permission.
Are you interested in testing overlayfs on top of NFS at this time? You need xattr support right? Stacking over NFS takes extra effort, esp. b/c of the pesky silly deleted files and needing a nameidata everywhere (but I managed after a while to get Wrapfs working stably over nfs3).
> Thanks,
> Miklos
>
> commit 9192816148e2c6b1d610226b1fc1c04c36216370
> Author: Miklos Szeredi <[email protected]>
> Date: Fri May 20 16:07:34 2011 +0200
>
> ovl: don't allow upperdentry to go negative
[...]
Do you commit such patches to a master branch, the v9 branch, or some other one; I'd like to be able to git pull
Thanks,
Erez.
Erez Zadok <[email protected]> writes:
> Done: reverted my ovl patches and applied yours. And indeed, the NULL
> oopses don't show up after 10 minutes of running. Just to be sure, I
> ran overlayfs on top of ext2, ext3, xfs, reiserfs, and tmps — no
> oopses. And I wasn't able to reproduce the WARN_ON in ovl_permission.
Great, thanks for testing. Pushed to .v9 branch.
> Are you interested in testing overlayfs on top of NFS at this time?
No, not as an upper layer. NFS as a lower layer should work fine.
Thanks,
Miklos