All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hin-Tak Leung <htl10@users.sourceforge.net>
To: Vyacheslav Dubeyko <slava@dubeyko.com>,
	Sergei Antonov <saproj@gmail.com>
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: A few thoughts on hfsplus dev (Re: [PATCH v2] hfsplus: add journal replay)
Date: Sun, 23 Mar 2014 23:15:38 +0000 (GMT)	[thread overview]
Message-ID: <1395616538.43957.YahooMailBasic@web172305.mail.ir2.yahoo.com> (raw)
In-Reply-To: <CABikg9y=FbG1YTEGhmVyqwp2BUs59_9v1z0TCtL9Wy78f8MKtw@mail.gmail.com>

Hi Vyacheslav and Sergei,

I was hoping not to feed into the issues. I believe the bottom line is that, there are
a few interesting and challenging issues with HFS+ support, and there are simply not
enough people who are both knowledgeable and motivated to improve on it. So we
should try to work together. Does it sound reasonable?

Sergei: - I appreciate that you are the first to point out the endian issue with the journal,
and been working on other aspects, such as the foldercount issue. I would be happy
to see more work from you, and continue to be willing to have a look and review, where
time permits. However, I do see a few problems with the manner in which you approach
this work. From my personal point of view (and I cannot speak for others), file system
work is data-safety first. So while Vyacheslav's work might be on the verbose side,
it is re-assuring to read, maybe repeatedly, about the details, than skipping on boring
details. I don't think length of code should be a criteria, and certainly not "elegance",
or cleverness. Here is a widely quoted saying from Brian Kernighan:
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the
code as cleverly as possible, you are, by definition, not smart enough to debug it."

- Also, if you deviate from how HFS+ works either at the specification level, the behavior level
or the code level, you are just implementing your own journalling work. I would also have
preferred that you point out exactly where Vyacheslav's work went wrong, and supply
a patch on top, instead of starting your own. If you want credit for it, I hope you can
arrange with Vyacheslav to swap the order of signed-off, for one or more of the merged patches,
for example. In any case, while I am happy to see your work, and am willing to review it,
you have not convinced me to try your work out yet. So.

Vyacheslav: Thanks for the continuous good work. While your adherence to TN1150 is
good, I have noticed a few times that you have not paid attention to Apple's
implementation of it - xnu's kernel code and diskdev_cmds (apple's mkfs/fsck). It is apparent
in some of the discussion between you and Sergei that he has read some of those but
you haven't. This is shown in the discussion about meaning and naming of variables,
and interpretation of reserved fields. About TN1150, I think there is one instance about
journalling support (a block count?) where it is ambiguous and contradicctory about a +1 offset.
Anyway, I think reference to the actual implementations are welcomed in resolving
these ambiguities and clarifying issues.

To the both of you, looking forward, and a few technical issues:

- I think fsck.hfsplus must manipulate the journal - fs with a non-empty journal is by definition
unclean; but fsck may or may not be doing journal replay. It probably does something else,
like just zero'ing the journal and fixes inconsistency in the rest. At some point we should find
out what fsck does when it fixes an unclean fs with non-empty journal.

- I have a more re-produce'able recipe of making disk images with strange 
location of 2nd volume header using hdiutil (Apple's loopback mounting, etc disk image
manipulation tool). At some point we should re-visit this issue.

- on the many-year-old issue of the driver getting confused just running "du" recursing
down large directories, I have managed to make it happen under Ftrace the Linux
kernel internal tracer, and even with code mod's, but the resulting log is too
large and events are being dropped. So I need to do some filtering to get
events down to a fully-recordable level. I.e. it would help to confirm issues
if I know where to look...  

Hin-Tak
 

  reply	other threads:[~2014-03-23 23:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-24 10:05 [PATCH v2] hfsplus: add journal replay Sergei Antonov
2014-02-24 10:21 ` Vyacheslav Dubeyko
2014-03-09 16:36 ` Vyacheslav Dubeyko
2014-03-09 22:50   ` Sergei Antonov
2014-03-13 10:20     ` Vyacheslav Dubeyko
2014-03-13 20:04       ` Sergei Antonov
2014-03-23 23:15         ` Hin-Tak Leung [this message]
2014-03-24  6:56           ` A few thoughts on hfsplus dev (Re: [PATCH v2] hfsplus: add journal replay) Vyacheslav Dubeyko
2014-03-25  1:37           ` Sergei Antonov
2014-03-25  6:30             ` Vyacheslav Dubeyko
2014-03-28 13:42               ` Sergei Antonov
2014-03-28 15:09                 ` Vyacheslav Dubeyko
2014-03-28 20:04                   ` Sergei Antonov
2014-04-04 23:35 Hin-Tak Leung
2014-04-06 20:59 ` Sergei Antonov
2014-04-05  1:02 Hin-Tak Leung
2014-04-06 16:01 ` Sergei Antonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1395616538.43957.YahooMailBasic@web172305.mail.ir2.yahoo.com \
    --to=htl10@users.sourceforge.net \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=saproj@gmail.com \
    --cc=slava@dubeyko.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.