[2.6-git PATCH] Fix for HFSPlus, should go in before 2.6.14!

[2.6-git PATCH] Fix for HFSPlus, should go in before 2.6.14!

[Anton Altaparmakov]

Hi Linus,

Roman Zippel the HFS/HFS+ maintainer seems to not be responding to email 
or lkml posts...

I reported a bug with HFS+ which causes deleting files not to free up the 
space they occupy randomly (at least when run on i386).

Peter Wainwright reported he experienced the same problem and he posted a 
fix.  His patch was corrupt so I re-did it against latest git, please find 
it below, together with Peter's explanation of the bug and fix.

Although I have not tested it myself yet (don't have the external hd with 
hfs+ here at work and at the moment it is formated with ntfs anyway), it 
is just a one liner and obviously correct.

Please apply before you release 2.6.14 if at all possible as HFS+ is 
seriously borked without it...  I have now given up waiting for Roman to 
reply given my original mail to him was two weeks ago and you are about 
to release 2.6.14...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

---

[PATCH] Fix HFS+ to free up the space when a file is deleted.

fsck_hfs reveals lots of temporary files accumulating in
the hidden directory "\000\000\000HFS+ Private Data".
According to the HFS+ documentation these are files which
are unlinked while in use.  However, there may be a bug in
the Linux hfsplus implementation which causes this to happen
even when the files are not in use. It looks like the
"opencnt" field is never initialized as (I think) it should
be in hfsplus_read_inode.  This means that a file can appear
to be still in use when in fact it has been closed. This patch
seems to fix it for me.

From: Peter Wainwright <peter.wainwright@hpa-rp.org.uk>
Signed-off-by: Anton Altaparmakov <aia21@cantab.net>

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index fd0f0f0..452fc1f 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -50,6 +50,7 @@ static void hfsplus_read_inode(struct in
 	init_MUTEX(&HFSPLUS_I(inode).extents_lock);
 	HFSPLUS_I(inode).flags = 0;
 	HFSPLUS_I(inode).rsrc_inode = NULL;
+	atomic_set(&HFSPLUS_I(inode).opencnt, 0);
 
 	if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID) {
 	read_inode:

Re: [2.6-git PATCH] Fix for HFSPlus, should go in before 2.6.14!

[Roman Zippel]

Hi,

On Wed, 26 Oct 2005, Anton Altaparmakov wrote:

> Please apply before you release 2.6.14 if at all possible as HFS+ is 
> seriously borked without it...  I have now given up waiting for Roman to 
> reply given my original mail to him was two weeks ago and you are about 
> to release 2.6.14...

I haven't responded yet, because I didn't have the time to verify and test 
the problem yet. It's not "seriously borked", it doesn't crash, it still 
works and the problem is easily fixed by fsck, so it wasn't on the top of 
my todo list.
The patch looks fine, although I'd probably prefered to move this kind of 
initialization into a separate function, as it's duplicated right now.

bye, Roman

Re: [2.6-git PATCH] Fix for HFSPlus, should go in before 2.6.14!

[Anton Altaparmakov]

Hi,

On Wed, 2005-10-26 at 10:59 +0200, Roman Zippel wrote:
> On Wed, 26 Oct 2005, Anton Altaparmakov wrote:
> 
> > Please apply before you release 2.6.14 if at all possible as HFS+ is 
> > seriously borked without it...  I have now given up waiting for Roman to 
> > reply given my original mail to him was two weeks ago and you are about 
> > to release 2.6.14...
> 
> I haven't responded yet, because I didn't have the time to verify and test 
> the problem yet. It's not "seriously borked", it doesn't crash, it still 
> works and the problem is easily fixed by fsck, so it wasn't on the top of 
> my todo list.

That is fair enough.  It is high on my TODO list so I sent the patch
bypassing you (and you were still CC:-ed so you could object if you
wanted).

I didn't even know there is an fsck for HFS+ (the one in hfsplusutils is
read-only so not much use - now I have found out pdisk contains it but
pdisk is not found on i386 distributions, at least not on SUSE 9.3 which
I was using at the time).

Nowhere in the kernel documentation (not in the Kconfig and not in
Documentation/filesystems anyway) is there anything about hfsplus.
There is a mention of hfsutils but not hfsplusutils or pdisk...  Perhaps
a documentation update is on order?

For me this bug meant I could no longer copy files to the disk and
wasted a day that way as it was full even though I deleted files to make
space and I had to take it back home to fix things on OSX and take it
back to work the next day to copy the files on Linux...

Anyway, requiring an fsck after every umount definitely fits my
definition of seriously borked.  YMMV.  (-:

> The patch looks fine, although I'd probably prefered to move this kind of 
> initialization into a separate function, as it's duplicated right now.

What is stopping you from doing so at some later point in time?

Whenever my git pull shows me that patches have hit fs/ntfs/ I look at
them and change them if I don't like something and make a new commit.
Andrew pulls my git repository for each -mm so it gets in that way to
-mm and I submit regular-ish updates to Linus for inclusion so for small
things like that they just get queued until a bigger submit.  Anyway,
that's just how I work.  (-:

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/