On 12 February 2014 18:55, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote: > > On Feb 12, 2014, at 7:42 PM, Sergei Antonov wrote: > >> On 12 February 2014 15:26, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote: >>> From: Vyacheslav Dubeyko <slava@xxxxxxxxxxx> >>> Subject: [PATCH v3 04/15] hfsplus: implement functionality for HFSPLUS_JOURNAL_NEED_INIT flag >>> >>> This patch implements functionality of creation of journal header, >>> journal buffer and initialization of them. >> >> Why initialize journal if you are not going to journal changes? It is >> probably better and safer to leave the "need initialize" flag. An >> implementation that really does journaling will initialize it on >> mount. >> > > Technical Note TN1150 > "kJIJournalNeedInitMask > This bit is set to indicate that the journal header is invalid and needs to be initialized. This bit is typically set when the journal is first created, and the space has been allocated; the first mount of the journaled volume typically initializes the journal header and clears this bit. When this bit is set, there are no valid transactions in the journal." Which does not mean initialization is necessary. It is, of course, necessary for drivers that write to journal because they need the journal to write transactions to. But in this driver you initialize it and ... what? Nothing. It will not be used. > >>> >>> + >>> + jnl->jh->magic = cpu_to_le32(HFSPLUS_JOURNAL_HEADER_MAGIC); >>> + jnl->jh->endian = cpu_to_le32(HFSPLUS_JOURNAL_HEADER_ENDIAN); >> >> It is not very important, but using native endianness (so that the >> journal will be either LE or BE) leads to shorter and more effective >> jounaling code (which does not exist at the moment though). >> > > Could you describe in more details your suggestion? jnl->jh->magic = HFSPLUS_JOURNAL_HEADER_MAGIC; jnl->jh->endian = HFSPLUS_JOURNAL_HEADER_ENDIAN; or do not initialze at all. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
