[v2] hfs,hfsplus: cache pages correctly between bnode_create and bnode_free
diff mbox

Message ID 1435455570-8577-2-git-send-email-HinTak.Leung@gmail.com
State New
Headers show

Commit Message

Hin-Tak Leung June 28, 2015, 1:39 a.m. UTC
From: Hin-Tak Leung <htl10@users.sourceforge.net>

Pages looked up by __hfs_bnode_create() (called by hfs_bnode_create() and
hfs_bnode_find() for finding or creating pages corresponding to an inode) are
immediately kmap()'ed and used (both read and write) and kunmap()'ed, and
should not be page_cache_release()'ed until hfs_bnode_free().

This patch fixes a problem I first saw in July 2012: merely running "du" on a
large hfsplus-mounted directory a few times on a reasonably loaded system would
get the hfsplus driver all confused and complaining about B-tree
inconsistencies, and generates a "BUG: Bad page state". Most recently, I can
generate this problem on up-to-date Fedora 22 with shipped kernel 4.0.5, by
running "du /" (="/" + "/home" + "/mnt" + other smaller mounts) and "du /mnt"
simultaneously on two windows, where /mnt is a lightly-used QEMU VM image of
the full Mac OS X 10.9:

$ df -i / /home /mnt
Filesystem                  Inodes   IUsed      IFree IUse% Mounted on
/dev/mapper/fedora-root    3276800  551665    2725135   17% /
/dev/mapper/fedora-home   52879360  716221   52163139    2% /home
/dev/nbd0p2             4294967295 1387818 4293579477    1% /mnt

After applying the patch, I was able to run "du /" (60+ times) and
"du /mnt" (150+ times) continuously and simultaneously for 6+ hours.

There are many reports of the hfsplus driver getting confused under load and
generating "BUG: Bad page state" or other similar issues over the years. [1]

The unpatched code [2] has always been wrong since it entered the kernel tree.
The only reason why it gets away with it is that the kmap/memcpy/kunmap follow
very quickly after the page_cache_release() so the kernel has not had a chance
to reuse the memory for something else, most of the time.

The current RW driver appears to have followed the design and development of
the earlier read-only hfsplus driver [3], where-by version 0.1 (Dec 2001) had a
B-tree node-centric approach to read_cache_page()/page_cache_release() per
bnode_get()/bnode_put(), migrating towards version 0.2 (June 2002) of caching
and releasing pages per inode extents. When the current RW code first entered
the kernel [2] in 2005, there was an REF_PAGES conditional (and "//" commented
out code) to switch between B-node centric paging to inode-centric paging.
There was a mistake with the direction of one of the REF_PAGES conditionals in
__hfs_bnode_create(). In a subsequent "remove debug code" commit [4], the
read_cache_page()/page_cache_release() per bnode_get()/bnode_put() were
removed, but a page_cache_release() was mistakenly left in (propagating the
"REF_PAGES <-> !REF_PAGE" mistake), and the commented-out page_cache_release()
in bnode_release() (which should be spanned by !REF_PAGES) was never enabled.

References:
[1]:
Michael Fox, Apr 2013
http://www.spinics.net/lists/linux-fsdevel/msg63807.html
("hfsplus volume suddenly inaccessable after 'hfs: recoff %d too large'")

Sasha Levin, Feb 2015
http://lkml.org/lkml/2015/2/20/85 ("use after free")

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/740814
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1027887
https://bugzilla.kernel.org/show_bug.cgi?id=42342
https://bugzilla.kernel.org/show_bug.cgi?id=63841
https://bugzilla.kernel.org/show_bug.cgi?id=78761

[2]:
http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
fs/hfs/bnode.c?id=d1081202f1d0ee35ab0beb490da4b65d4bc763db
commit d1081202f1d0ee35ab0beb490da4b65d4bc763db
Author: Andrew Morton <akpm@osdl.org>
Date:   Wed Feb 25 16:17:36 2004 -0800

    [PATCH] HFS rewrite

http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
fs/hfsplus/bnode.c?id=91556682e0bf004d98a529bf829d339abb98bbbd

commit 91556682e0bf004d98a529bf829d339abb98bbbd
Author: Andrew Morton <akpm@osdl.org>
Date:   Wed Feb 25 16:17:48 2004 -0800

    [PATCH] HFS+ support

[3]:
http://sourceforge.net/projects/linux-hfsplus/

http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.1/
http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.2/

http://linux-hfsplus.cvs.sourceforge.net/viewvc/linux-hfsplus/linux/\
fs/hfsplus/bnode.c?r1=1.4&r2=1.5

Date:   Thu Jun 6 09:45:14 2002 +0000
Use buffer cache instead of page cache in bnode.c. Cache inode extents.

[4]:
http://git.kernel.org/cgit/linux/kernel/git/\
stable/linux-stable.git/commit/?id=a5e3985fa014029eb6795664c704953720cc7f7d

commit a5e3985fa014029eb6795664c704953720cc7f7d
Author: Roman Zippel <zippel@linux-m68k.org>
Date:   Tue Sep 6 15:18:47 2005 -0700

[PATCH] hfs: remove debug code

Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
Reviewed-by: Anton Altaparmakov <anton@tuxera.com>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Sougata Santra <sougata@tuxera.com>
---
 fs/hfs/bnode.c     | 9 ++++-----
 fs/hfsplus/bnode.c | 3 ---
 2 files changed, 4 insertions(+), 8 deletions(-)

Comments

Sergei Antonov June 28, 2015, 6:52 p.m. UTC | #1
On 28 June 2015 at 03:39, Hin-Tak Leung <hintak.leung@gmail.com> wrote:
> From: Hin-Tak Leung <htl10@users.sourceforge.net>
>
> Pages looked up by __hfs_bnode_create() (called by hfs_bnode_create() and
> hfs_bnode_find() for finding or creating pages corresponding to an inode) are
> immediately kmap()'ed and used (both read and write) and kunmap()'ed, and
> should not be page_cache_release()'ed until hfs_bnode_free().
>
> This patch fixes a problem I first saw in July 2012: merely running "du" on a
> large hfsplus-mounted directory a few times on a reasonably loaded system would
> get the hfsplus driver all confused and complaining about B-tree
> inconsistencies, and generates a "BUG: Bad page state". Most recently, I can
> generate this problem on up-to-date Fedora 22 with shipped kernel 4.0.5, by
> running "du /" (="/" + "/home" + "/mnt" + other smaller mounts) and "du /mnt"
> simultaneously on two windows, where /mnt is a lightly-used QEMU VM image of
> the full Mac OS X 10.9:
>
> $ df -i / /home /mnt
> Filesystem                  Inodes   IUsed      IFree IUse% Mounted on
> /dev/mapper/fedora-root    3276800  551665    2725135   17% /
> /dev/mapper/fedora-home   52879360  716221   52163139    2% /home
> /dev/nbd0p2             4294967295 1387818 4293579477    1% /mnt
>
> After applying the patch, I was able to run "du /" (60+ times) and
> "du /mnt" (150+ times) continuously and simultaneously for 6+ hours.
>
> There are many reports of the hfsplus driver getting confused under load and
> generating "BUG: Bad page state" or other similar issues over the years. [1]
>
> The unpatched code [2] has always been wrong since it entered the kernel tree.
> The only reason why it gets away with it is that the kmap/memcpy/kunmap follow
> very quickly after the page_cache_release() so the kernel has not had a chance
> to reuse the memory for something else, most of the time.
>
> The current RW driver appears to have followed the design and development of
> the earlier read-only hfsplus driver [3], where-by version 0.1 (Dec 2001) had a
> B-tree node-centric approach to read_cache_page()/page_cache_release() per
> bnode_get()/bnode_put(), migrating towards version 0.2 (June 2002) of caching
> and releasing pages per inode extents. When the current RW code first entered
> the kernel [2] in 2005, there was an REF_PAGES conditional (and "//" commented
> out code) to switch between B-node centric paging to inode-centric paging.
> There was a mistake with the direction of one of the REF_PAGES conditionals in
> __hfs_bnode_create(). In a subsequent "remove debug code" commit [4], the
> read_cache_page()/page_cache_release() per bnode_get()/bnode_put() were
> removed, but a page_cache_release() was mistakenly left in (propagating the
> "REF_PAGES <-> !REF_PAGE" mistake), and the commented-out page_cache_release()
> in bnode_release() (which should be spanned by !REF_PAGES) was never enabled.
>
> References:
> [1]:
> Michael Fox, Apr 2013
> http://www.spinics.net/lists/linux-fsdevel/msg63807.html
> ("hfsplus volume suddenly inaccessable after 'hfs: recoff %d too large'")
>
> Sasha Levin, Feb 2015
> http://lkml.org/lkml/2015/2/20/85 ("use after free")
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/740814
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1027887
> https://bugzilla.kernel.org/show_bug.cgi?id=42342
> https://bugzilla.kernel.org/show_bug.cgi?id=63841
> https://bugzilla.kernel.org/show_bug.cgi?id=78761
>
> [2]:
> http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
> fs/hfs/bnode.c?id=d1081202f1d0ee35ab0beb490da4b65d4bc763db
> commit d1081202f1d0ee35ab0beb490da4b65d4bc763db
> Author: Andrew Morton <akpm@osdl.org>
> Date:   Wed Feb 25 16:17:36 2004 -0800
>
>     [PATCH] HFS rewrite
>
> http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
> fs/hfsplus/bnode.c?id=91556682e0bf004d98a529bf829d339abb98bbbd
>
> commit 91556682e0bf004d98a529bf829d339abb98bbbd
> Author: Andrew Morton <akpm@osdl.org>
> Date:   Wed Feb 25 16:17:48 2004 -0800
>
>     [PATCH] HFS+ support
>
> [3]:
> http://sourceforge.net/projects/linux-hfsplus/
>
> http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.1/
> http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.2/
>
> http://linux-hfsplus.cvs.sourceforge.net/viewvc/linux-hfsplus/linux/\
> fs/hfsplus/bnode.c?r1=1.4&r2=1.5
>
> Date:   Thu Jun 6 09:45:14 2002 +0000
> Use buffer cache instead of page cache in bnode.c. Cache inode extents.
>
> [4]:
> http://git.kernel.org/cgit/linux/kernel/git/\
> stable/linux-stable.git/commit/?id=a5e3985fa014029eb6795664c704953720cc7f7d
>
> commit a5e3985fa014029eb6795664c704953720cc7f7d
> Author: Roman Zippel <zippel@linux-m68k.org>
> Date:   Tue Sep 6 15:18:47 2005 -0700
>
> [PATCH] hfs: remove debug code
>
> Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> Reviewed-by: Anton Altaparmakov <anton@tuxera.com>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
> Cc: Sougata Santra <sougata@tuxera.com>
> ---
>  fs/hfs/bnode.c     | 9 ++++-----
>  fs/hfsplus/bnode.c | 3 ---
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index d3fa6bd..221719e 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -288,7 +288,6 @@ static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
>                         page_cache_release(page);
>                         goto fail;
>                 }
> -               page_cache_release(page);
>                 node->page[i] = page;
>         }
>
> @@ -398,11 +397,11 @@ node_error:
>
>  void hfs_bnode_free(struct hfs_bnode *node)
>  {
> -       //int i;
> +       int i;
>
> -       //for (i = 0; i < node->tree->pages_per_bnode; i++)
> -       //      if (node->page[i])
> -       //              page_cache_release(node->page[i]);
> +       for (i = 0; i < node->tree->pages_per_bnode; i++)
> +               if (node->page[i])
> +                       page_cache_release(node->page[i]);
>         kfree(node);
>  }
>
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index 759708f..6392466 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -454,7 +454,6 @@ static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
>                         page_cache_release(page);
>                         goto fail;
>                 }
> -               page_cache_release(page);
>                 node->page[i] = page;
>         }
>
> @@ -566,13 +565,11 @@ node_error:
>
>  void hfs_bnode_free(struct hfs_bnode *node)
>  {
> -#if 0
>         int i;
>
>         for (i = 0; i < node->tree->pages_per_bnode; i++)
>                 if (node->page[i])
>                         page_cache_release(node->page[i]);
> -#endif
>         kfree(node);
>  }
>
> --
> 2.4.3

If I fix something else in hfsplus in the future, will you again
submit a combined hfsplus+hfs patch? I would prefer separation. Hoped
to receive your "Tested-by:" for my "hfsplus: release bnode pages
after use, not before" and then submit a V2 of it with a longer
description.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hin-Tak Leung June 30, 2015, 3:40 p.m. UTC | #2
On 28 June 2015 at 19:52, Sergei Antonov <saproj@gmail.com> wrote:
<snipped>
> If I fix something else in hfsplus in the future, will you again
> submit a combined hfsplus+hfs patch? I would prefer separation. Hoped
> to receive your "Tested-by:" for my "hfsplus: release bnode pages
> after use, not before" and then submit a V2 of it with a longer
> description.

Possibly yes, if the patch description is clearly unsatisfactory and
deemed incomprehensible, and you have not re-submitted a v2
within a reasonable time. I already explained why I re-submitted
with a different patch description in the first of 3 below:

[PATCH 0/2] two patches about B-tree corruptions in hfs and hfsplus
[PATCH v2] hfs,hfsplus: cache pages correctly between bnode_create and
[PATCH] hfs: fix B-tree corruption after insertion at position 0

Please just re-submit v2 yourself if more than a few people thinks your patch
description is unsatisfactory, instead of waiting for somebody else to
do it for you;
and also please just say "thank you", when others are willing spend their
valuable time to look at and check and verify what you do.

I would not be willing to put my name down with a Test-By: or Signed-of:
on your original patch as it was.

Hin-Tak
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Antonov July 1, 2015, 4:09 p.m. UTC | #3
On 30 June 2015 at 17:40, Hin-Tak Leung <hintak.leung@gmail.com> wrote:
> On 28 June 2015 at 19:52, Sergei Antonov <saproj@gmail.com> wrote:
> <snipped>
>> If I fix something else in hfsplus in the future, will you again
>> submit a combined hfsplus+hfs patch? I would prefer separation. Hoped
>> to receive your "Tested-by:" for my "hfsplus: release bnode pages
>> after use, not before" and then submit a V2 of it with a longer
>> description.
>
> Possibly yes, if the patch description is clearly unsatisfactory and
> deemed incomprehensible, and you have not re-submitted a v2
> within a reasonable time. I already explained why I re-submitted
> with a different patch description in the first of 3 below:
>
> [PATCH 0/2] two patches about B-tree corruptions in hfs and hfsplus
> [PATCH v2] hfs,hfsplus: cache pages correctly between bnode_create and
> [PATCH] hfs: fix B-tree corruption after insertion at position 0
>
> Please just re-submit v2 yourself if more than a few people thinks your patch
> description is unsatisfactory, instead of waiting for somebody else to
> do it for you;
> and also please just say "thank you", when others are willing spend their
> valuable time to look at and check and verify what you do.

This "what I do" fixes the problem you have been complaining about for
years. The historical research you have done is interesting, but
simple testing is to be expected in the first place.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hin-Tak Leung July 1, 2015, 11:24 p.m. UTC | #4
On 1 July 2015 at 17:09, Sergei Antonov <saproj@gmail.com> wrote:
> On 30 June 2015 at 17:40, Hin-Tak Leung <hintak.leung@gmail.com> wrote:
>> On 28 June 2015 at 19:52, Sergei Antonov <saproj@gmail.com> wrote:
>> <snipped>
>>> If I fix something else in hfsplus in the future, will you again
>>> submit a combined hfsplus+hfs patch? I would prefer separation. Hoped
>>> to receive your "Tested-by:" for my "hfsplus: release bnode pages
>>> after use, not before" and then submit a V2 of it with a longer
>>> description.
>>
>> Possibly yes, if the patch description is clearly unsatisfactory and
>> deemed incomprehensible, and you have not re-submitted a v2
>> within a reasonable time. I already explained why I re-submitted
>> with a different patch description in the first of 3 below:
>>
>> [PATCH 0/2] two patches about B-tree corruptions in hfs and hfsplus
>> [PATCH v2] hfs,hfsplus: cache pages correctly between bnode_create and
>> [PATCH] hfs: fix B-tree corruption after insertion at position 0
>>
>> Please just re-submit v2 yourself if more than a few people thinks your patch
>> description is unsatisfactory, instead of waiting for somebody else to
>> do it for you;
>> and also please just say "thank you", when others are willing spend their
>> valuable time to look at and check and verify what you do.
>
> This "what I do" fixes the problem you have been complaining about for
> years. The historical research you have done is interesting, but
> simple testing is to be expected in the first place.

You are still trying to argue that your patch description is not poor,
as you did a few times in this thread already. Quite a few of us had
already said
it is poor. I did not think you were going to submit a v2, because you
have not really accepted any criticisms as valid, either.

You don't think the lost development history between 2001
and 2005 is important. I think it is. That is clearly reflected in how poor
your patch description is, and why I re-wrote the patch description.
Not new idea here.

I am still waiting for that 'thank you' for time spent on testing and collating
all the discussion into the new patch description.

Hin-Tak
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Antonov July 2, 2015, 5:04 p.m. UTC | #5
On 2 July 2015 at 01:24, Hin-Tak Leung <hintak.leung@gmail.com> wrote:
> On 1 July 2015 at 17:09, Sergei Antonov <saproj@gmail.com> wrote:
>> On 30 June 2015 at 17:40, Hin-Tak Leung <hintak.leung@gmail.com> wrote:
>>> On 28 June 2015 at 19:52, Sergei Antonov <saproj@gmail.com> wrote:
>>> <snipped>
>>>> If I fix something else in hfsplus in the future, will you again
>>>> submit a combined hfsplus+hfs patch? I would prefer separation. Hoped
>>>> to receive your "Tested-by:" for my "hfsplus: release bnode pages
>>>> after use, not before" and then submit a V2 of it with a longer
>>>> description.
>>>
>>> Possibly yes, if the patch description is clearly unsatisfactory and
>>> deemed incomprehensible, and you have not re-submitted a v2
>>> within a reasonable time. I already explained why I re-submitted
>>> with a different patch description in the first of 3 below:
>>>
>>> [PATCH 0/2] two patches about B-tree corruptions in hfs and hfsplus
>>> [PATCH v2] hfs,hfsplus: cache pages correctly between bnode_create and
>>> [PATCH] hfs: fix B-tree corruption after insertion at position 0
>>>
>>> Please just re-submit v2 yourself if more than a few people thinks your patch
>>> description is unsatisfactory, instead of waiting for somebody else to
>>> do it for you;
>>> and also please just say "thank you", when others are willing spend their
>>> valuable time to look at and check and verify what you do.
>>
>> This "what I do" fixes the problem you have been complaining about for
>> years. The historical research you have done is interesting, but
>> simple testing is to be expected in the first place.
>
> You are still trying to argue that your patch description is not poor,
> as you did a few times in this thread already. Quite a few of us had
> already said
> it is poor. I did not think you were going to submit a v2, because you
> have not really accepted any criticisms as valid, either.
>
> You don't think the lost development history between 2001
> and 2005 is important. I think it is. That is clearly reflected in how poor
> your patch description is, and why I re-wrote the patch description.
> Not new idea here.
>
> I am still waiting for that 'thank you' for time spent on testing and collating
> all the discussion into the new patch description.


Just sent a V2 of my patch. Took three links to previous bug reports
found by you. Thank you for them!
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hin-Tak Leung July 2, 2015, 6:02 p.m. UTC | #6
On 2 July 2015 at 18:04, Sergei Antonov <saproj@gmail.com> wrote:
<snipped>
>
>
> Just sent a V2 of my patch. Took three links to previous bug reports
> found by you. Thank you for them!

Your v2 is still poor, and you have not taken much review on board.
I am getting tired of this not-discussion.

I'd like to see who in the CC'ed want to Acked-By/Reviewed-By/Signed-off
on which one to go into the kernel.

Hin-Tak
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Morton July 7, 2015, 10:19 p.m. UTC | #7
On Thu, 2 Jul 2015 19:02:15 +0100 Hin-Tak Leung <hintak.leung@gmail.com> wrote:

> On 2 July 2015 at 18:04, Sergei Antonov <saproj@gmail.com> wrote:
> <snipped>
> >
> >
> > Just sent a V2 of my patch. Took three links to previous bug reports
> > found by you. Thank you for them!
> 
> Your v2 is still poor, and you have not taken much review on board.
> I am getting tired of this not-discussion.
> 
> I'd like to see who in the CC'ed want to Acked-By/Reviewed-By/Signed-off
> on which one to go into the kernel.
> 

c'mon guys, this drama isn't helping.

I merged Hin-Tak's two patches based on Vyacheslav's recommendation and
because I like elaborate changelogs.

Sergei, should I convert Cc:you into Signed-off-by:you?

I added cc:stable to both patches.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Antonov July 7, 2015, 11:19 p.m. UTC | #8
On 8 July 2015 at 00:19, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 2 Jul 2015 19:02:15 +0100 Hin-Tak Leung <hintak.leung@gmail.com> wrote:
>
>> On 2 July 2015 at 18:04, Sergei Antonov <saproj@gmail.com> wrote:
>> <snipped>
>> >
>> >
>> > Just sent a V2 of my patch. Took three links to previous bug reports
>> > found by you. Thank you for them!
>>
>> Your v2 is still poor, and you have not taken much review on board.
>> I am getting tired of this not-discussion.
>>
>> I'd like to see who in the CC'ed want to Acked-By/Reviewed-By/Signed-off
>> on which one to go into the kernel.
>>
>
> c'mon guys, this drama isn't helping.
>
> I merged Hin-Tak's two patches based on Vyacheslav's recommendation and
> because I like elaborate changelogs.
>
> Sergei, should I convert Cc:you into Signed-off-by:you?

No, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Antonov July 8, 2015, 2:58 p.m. UTC | #9
On 8 July 2015 at 00:19, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 2 Jul 2015 19:02:15 +0100 Hin-Tak Leung <hintak.leung@gmail.com> wrote:
>
>> On 2 July 2015 at 18:04, Sergei Antonov <saproj@gmail.com> wrote:
>> <snipped>
>> >
>> >
>> > Just sent a V2 of my patch. Took three links to previous bug reports
>> > found by you. Thank you for them!
>>
>> Your v2 is still poor, and you have not taken much review on board.
>> I am getting tired of this not-discussion.
>>
>> I'd like to see who in the CC'ed want to Acked-By/Reviewed-By/Signed-off
>> on which one to go into the kernel.
>>
>
> c'mon guys, this drama isn't helping.

Can't help mentioning it: this drama reminds me that of Grisha
Perelman's and Shing-Tung Yau's.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hin-Tak Leung July 8, 2015, 3:29 p.m. UTC | #10
On 8 July 2015 at 15:58, Sergei Antonov <saproj@gmail.com> wrote:
> On 8 July 2015 at 00:19, Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Thu, 2 Jul 2015 19:02:15 +0100 Hin-Tak Leung <hintak.leung@gmail.com> wrote:
>>
>>> On 2 July 2015 at 18:04, Sergei Antonov <saproj@gmail.com> wrote:
>>> <snipped>
>>> >
>>> >
>>> > Just sent a V2 of my patch. Took three links to previous bug reports
>>> > found by you. Thank you for them!
>>>
>>> Your v2 is still poor, and you have not taken much review on board.
>>> I am getting tired of this not-discussion.
>>>
>>> I'd like to see who in the CC'ed want to Acked-By/Reviewed-By/Signed-off
>>> on which one to go into the kernel.
>>>
>>
>> c'mon guys, this drama isn't helping.
>
> Can't help mentioning it: this drama reminds me that of Grisha
> Perelman's and Shing-Tung Yau's.

I do not know of either of those individuals, and just google'd them
and learned that they are mathematicians of some sort, the latter
obviously chinese, the former, apparently russian.

I would really prefer that you do not try to make any points
about others' cultural/ethnic origins. You are not endearing yourself
to others by making any points other than quiet acceptance
of others' cultural/ethnic origins. That sort of behaviour is called
"bigotry", and frowned upon in most modern civilized societies.
Please do not do that.

FWIW, my usual response to "are you chinese?" from an unknown
chinese person in the street, is usually either "no", or pretend
that I do not understand the question. In any case, it is irrelevant and
none of anybody's business, including you.

Hin-Tak
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luc Pionchon Aug. 3, 2015, 1:33 p.m. UTC | #11
On 8 July 2015 at 00:19, Andrew Morton <akpm@linux-foundation.org> wrote:
>

[snip]

>
> I merged Hin-Tak's two patches based on Vyacheslav's recommendation and
> because I like elaborate changelogs.

into which kernels were the patches applied ?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index d3fa6bd..221719e 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -288,7 +288,6 @@  static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
 			page_cache_release(page);
 			goto fail;
 		}
-		page_cache_release(page);
 		node->page[i] = page;
 	}
 
@@ -398,11 +397,11 @@  node_error:
 
 void hfs_bnode_free(struct hfs_bnode *node)
 {
-	//int i;
+	int i;
 
-	//for (i = 0; i < node->tree->pages_per_bnode; i++)
-	//	if (node->page[i])
-	//		page_cache_release(node->page[i]);
+	for (i = 0; i < node->tree->pages_per_bnode; i++)
+		if (node->page[i])
+			page_cache_release(node->page[i]);
 	kfree(node);
 }
 
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 759708f..6392466 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -454,7 +454,6 @@  static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
 			page_cache_release(page);
 			goto fail;
 		}
-		page_cache_release(page);
 		node->page[i] = page;
 	}
 
@@ -566,13 +565,11 @@  node_error:
 
 void hfs_bnode_free(struct hfs_bnode *node)
 {
-#if 0
 	int i;
 
 	for (i = 0; i < node->tree->pages_per_bnode; i++)
 		if (node->page[i])
 			page_cache_release(node->page[i]);
-#endif
 	kfree(node);
 }