Re: [PATCH v2] hfsplus: add HFSX subfolder count support

[Vyacheslav Dubeyko <slava@dubeyko.com>]

On Sat, 2014-02-08 at 10:17 +0100, Sergei Antonov wrote:
> On 7 February 2014 22:22, Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Fri,  7 Feb 2014 20:35:59 +0100 Sergei Antonov <saproj@gmail.com> wrote:
> >
> >> This patch adds support for HFSX 'HasFolderCount' flag and a corresponding
> >> 'folderCount' field in folder records. (For reference see HFS_FOLDERCOUNT
> >> and kHFSHasFolderCountBit/kHFSHasFolderCountMask in Apple's source code.)
> >>
> [skip]
> >
> > Two of hfsplus_rename_cat()'s callers hold vh_mutex, hfsplus_rename()
> > does not.  Which lock is preventing the obvious races here?
> 
> Each inode has its own counter and every counter increment and
> decrement is done while inode->i_mutex is locked.
> 

The vh_mutex is held by hfsplus_rmdir() and hfsplus_unlink in the
hfsplus_rename() method:

        if (new_dentry->d_inode) {
                if (S_ISDIR(new_dentry->d_inode->i_mode))
                        res = hfsplus_rmdir(new_dir, new_dentry);
                else
                        res = hfsplus_unlink(new_dir, new_dentry);
                if (res)
                        return res;
        }

These methods can modify shared superblock's fields. 

The hfsplus_rename_cat() method operates with inodes and CatalogFile's
metadata only. Before any operations inside CatalogFile we need to find
position of searching record. And it is made hfs_find_init() call before
such operation. This call locks btree's mutex on the whole operation
with the tree. Finally, this mutex is unlocked in hfs_find_exit() call.

But, as Sergei is said yet, added in the patch "subfolders" field is the
count of child folders inside parent one. So, this counter lives inside
inode (folder record of CatalogFile on the volume).

Thanks,
Vyacheslav Dubeyko.