On Mon, Oct 15, 2018 at 02:02:01PM -0700, Viacheslav Dubeyko wrote:
> > 1) unlink() finds the victim and locks it
> >
> > 2) in hfsplus_file_release():
> > if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
> > got to 0
> > inode_lock(inode);
> > block waiting for unlink
> >
> > 3) open() finds the sucker in dcache and hits hfsplus_file_open(), where
> > we do
> > atomic_inc(&HFSPLUS_I(inode)->opencnt);
> > and now opencnt is 1.
> >
> > 4) on the unlink side:
> > if (inode->i_ino == cnid &&
> > atomic_read(&HFSPLUS_I(inode)->opencnt)) {
> > str.name = name;
> > str.len = sprintf(name, "temp%lu", inode->i_ino);
> > res = hfsplus_rename_cat(inode->i_ino,
> > dir, &dentry->d_name,
> > sbi->hidden_dir, &str);
> > if (!res) {
> > inode->i_flags |= S_DEAD;
> > drop_nlink(inode);
> > }
> > goto out;
> > }
> > nlink is zero now, the sucker got renamed and marked S_DEAD
> >
> > 5) ->release() finally got through inode_lock() and
> > hfsplus_file_truncate(inode);
> > if (inode->i_flags & S_DEAD) {
> > hfsplus_delete_cat(inode->i_ino,
> > HFSPLUS_SB(sb)->hidden_dir, NULL);
> > hfsplus_delete_inode(inode);
> > }
> > inode_unlock(inode);
> > ... and now we have killed everything we used to have associated with that
> > inode on disk. While it's still open. What's to stop CNID to be reused,
> > etc. and what's to preserve sanity in that situation?
> >
> > What am I missing there?
>
> OK. If you believe that the patch is in the bad shape then what's your
> suggestion for improving the patch?
Tha patch is OK; the problem, AFAICS, is neither introduced nor fixed by it.
I might be wrong regarding the locking problem I described, but it does
appear to be real and I'd like somebody more familiar with HFS+ to comment
on it.
