* [PATCH 1/4] jbd2: Remove some unnecessary arguments of jbd2_journal_write_revoke_records
2016-01-07 14:41 [PATCH 0/4] JBD2 cleanups Jan Kara
@ 2016-01-07 14:41 ` Jan Kara
2016-02-23 4:09 ` Theodore Ts'o
2016-01-07 14:41 ` [PATCH 2/4] jbd2: Factor out common descriptor block initialization Jan Kara
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2016-01-07 14:41 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, Jan Kara
jbd2_journal_write_revoke_records() takes journal pointer and write_op,
although journal can be obtained from the passed transaction and
write_op is always WRITE_SYNC. Remove these superfluous arguments.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/commit.c | 3 +--
fs/jbd2/revoke.c | 33 +++++++++++++++------------------
include/linux/jbd2.h | 6 ++----
3 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 36345fefa3ff..ae4402d15d46 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -554,8 +554,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
jbd2_journal_abort(journal, err);
blk_start_plug(&plug);
- jbd2_journal_write_revoke_records(journal, commit_transaction,
- &log_bufs, WRITE_SYNC);
+ jbd2_journal_write_revoke_records(commit_transaction, &log_bufs);
jbd_debug(3, "JBD2: commit phase 2b\n");
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index 705ae577882b..c839332be56b 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -122,11 +122,11 @@ struct jbd2_revoke_table_s
#ifdef __KERNEL__
-static void write_one_revoke_record(journal_t *, transaction_t *,
+static void write_one_revoke_record(transaction_t *,
struct list_head *,
struct buffer_head **, int *,
- struct jbd2_revoke_record_s *, int);
-static void flush_descriptor(journal_t *, struct buffer_head *, int, int);
+ struct jbd2_revoke_record_s *);
+static void flush_descriptor(journal_t *, struct buffer_head *, int);
#endif
/* Utility functions to maintain the revoke table */
@@ -519,11 +519,10 @@ void jbd2_journal_switch_revoke_table(journal_t *journal)
* Write revoke records to the journal for all entries in the current
* revoke hash, deleting the entries as we go.
*/
-void jbd2_journal_write_revoke_records(journal_t *journal,
- transaction_t *transaction,
- struct list_head *log_bufs,
- int write_op)
+void jbd2_journal_write_revoke_records(transaction_t *transaction,
+ struct list_head *log_bufs)
{
+ journal_t *journal = transaction->t_journal;
struct buffer_head *descriptor;
struct jbd2_revoke_record_s *record;
struct jbd2_revoke_table_s *revoke;
@@ -544,16 +543,15 @@ void jbd2_journal_write_revoke_records(journal_t *journal,
while (!list_empty(hash_list)) {
record = (struct jbd2_revoke_record_s *)
hash_list->next;
- write_one_revoke_record(journal, transaction, log_bufs,
- &descriptor, &offset,
- record, write_op);
+ write_one_revoke_record(transaction, log_bufs,
+ &descriptor, &offset, record);
count++;
list_del(&record->hash);
kmem_cache_free(jbd2_revoke_record_cache, record);
}
}
if (descriptor)
- flush_descriptor(journal, descriptor, offset, write_op);
+ flush_descriptor(journal, descriptor, offset);
jbd_debug(1, "Wrote %d revoke records\n", count);
}
@@ -562,14 +560,13 @@ void jbd2_journal_write_revoke_records(journal_t *journal,
* block if the old one is full or if we have not already created one.
*/
-static void write_one_revoke_record(journal_t *journal,
- transaction_t *transaction,
+static void write_one_revoke_record(transaction_t *transaction,
struct list_head *log_bufs,
struct buffer_head **descriptorp,
int *offsetp,
- struct jbd2_revoke_record_s *record,
- int write_op)
+ struct jbd2_revoke_record_s *record)
{
+ journal_t *journal = transaction->t_journal;
int csum_size = 0;
struct buffer_head *descriptor;
int sz, offset;
@@ -597,7 +594,7 @@ static void write_one_revoke_record(journal_t *journal,
/* Make sure we have a descriptor with space left for the record */
if (descriptor) {
if (offset + sz > journal->j_blocksize - csum_size) {
- flush_descriptor(journal, descriptor, offset, write_op);
+ flush_descriptor(journal, descriptor, offset);
descriptor = NULL;
}
}
@@ -654,7 +651,7 @@ static void jbd2_revoke_csum_set(journal_t *j, struct buffer_head *bh)
static void flush_descriptor(journal_t *journal,
struct buffer_head *descriptor,
- int offset, int write_op)
+ int offset)
{
jbd2_journal_revoke_header_t *header;
@@ -670,7 +667,7 @@ static void flush_descriptor(journal_t *journal,
set_buffer_jwrite(descriptor);
BUFFER_TRACE(descriptor, "write");
set_buffer_dirty(descriptor);
- write_dirty_buffer(descriptor, write_op);
+ write_dirty_buffer(descriptor, WRITE_SYNC);
}
#endif
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 65407f6c9120..5716d9554e77 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1327,10 +1327,8 @@ extern int jbd2_journal_init_revoke_caches(void);
extern void jbd2_journal_destroy_revoke(journal_t *);
extern int jbd2_journal_revoke (handle_t *, unsigned long long, struct buffer_head *);
extern int jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
-extern void jbd2_journal_write_revoke_records(journal_t *journal,
- transaction_t *transaction,
- struct list_head *log_bufs,
- int write_op);
+extern void jbd2_journal_write_revoke_records(transaction_t *transaction,
+ struct list_head *log_bufs);
/* Recovery revoke support */
extern int jbd2_journal_set_revoke(journal_t *, unsigned long long, tid_t);
--
2.6.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/4] jbd2: Factor out common descriptor block initialization
2016-01-07 14:41 [PATCH 0/4] JBD2 cleanups Jan Kara
2016-01-07 14:41 ` [PATCH 1/4] jbd2: Remove some unnecessary arguments of jbd2_journal_write_revoke_records Jan Kara
@ 2016-01-07 14:41 ` Jan Kara
2016-02-23 4:18 ` Theodore Ts'o
2016-01-07 14:41 ` [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling Jan Kara
2016-01-07 14:41 ` [PATCH 4/4] jbd2: Save some atomic ops in __JI_COMMIT_RUNNING handling Jan Kara
3 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2016-01-07 14:41 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, Jan Kara
Descriptor block header is initialized in several places. Factor out the
common code into jbd2_journal_get_descriptor_buffer().
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/commit.c | 16 +++++-----------
fs/jbd2/journal.c | 9 ++++++++-
fs/jbd2/revoke.c | 8 ++------
include/linux/jbd2.h | 2 +-
4 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index ae4402d15d46..cf221f3d955a 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -131,14 +131,12 @@ static int journal_submit_commit_record(journal_t *journal,
if (is_journal_aborted(journal))
return 0;
- bh = jbd2_journal_get_descriptor_buffer(journal);
+ bh = jbd2_journal_get_descriptor_buffer(commit_transaction,
+ JBD2_COMMIT_BLOCK);
if (!bh)
return 1;
tmp = (struct commit_header *)bh->b_data;
- tmp->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
- tmp->h_blocktype = cpu_to_be32(JBD2_COMMIT_BLOCK);
- tmp->h_sequence = cpu_to_be32(commit_transaction->t_tid);
tmp->h_commit_sec = cpu_to_be64(now.tv_sec);
tmp->h_commit_nsec = cpu_to_be32(now.tv_nsec);
@@ -379,7 +377,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
ktime_t start_time;
u64 commit_time;
char *tagp = NULL;
- journal_header_t *header;
journal_block_tag_t *tag = NULL;
int space_left = 0;
int first_tag = 0;
@@ -615,7 +612,9 @@ void jbd2_journal_commit_transaction(journal_t *journal)
jbd_debug(4, "JBD2: get descriptor\n");
- descriptor = jbd2_journal_get_descriptor_buffer(journal);
+ descriptor = jbd2_journal_get_descriptor_buffer(
+ commit_transaction,
+ JBD2_DESCRIPTOR_BLOCK);
if (!descriptor) {
jbd2_journal_abort(journal, -EIO);
continue;
@@ -624,11 +623,6 @@ void jbd2_journal_commit_transaction(journal_t *journal)
jbd_debug(4, "JBD2: got buffer %llu (%p)\n",
(unsigned long long)descriptor->b_blocknr,
descriptor->b_data);
- header = (journal_header_t *)descriptor->b_data;
- header->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
- header->h_blocktype = cpu_to_be32(JBD2_DESCRIPTOR_BLOCK);
- header->h_sequence = cpu_to_be32(commit_transaction->t_tid);
-
tagp = &descriptor->b_data[sizeof(journal_header_t)];
space_left = descriptor->b_size -
sizeof(journal_header_t);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 81e622681c82..28d05bd9a588 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -805,10 +805,13 @@ int jbd2_journal_bmap(journal_t *journal, unsigned long blocknr,
* But we don't bother doing that, so there will be coherency problems with
* mmaps of blockdevs which hold live JBD-controlled filesystems.
*/
-struct buffer_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
+struct buffer_head *
+jbd2_journal_get_descriptor_buffer(transaction_t *transaction, int type)
{
+ journal_t *journal = transaction->t_journal;
struct buffer_head *bh;
unsigned long long blocknr;
+ journal_header_t *header;
int err;
err = jbd2_journal_next_log_block(journal, &blocknr);
@@ -821,6 +824,10 @@ struct buffer_head *jbd2_journal_get_descriptor_buffer(journal_t *journal)
return NULL;
lock_buffer(bh);
memset(bh->b_data, 0, journal->j_blocksize);
+ header = (journal_header_t *)bh->b_data;
+ header->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
+ header->h_blocktype = cpu_to_be32(type);
+ header->h_sequence = cpu_to_be32(transaction->t_tid);
set_buffer_uptodate(bh);
unlock_buffer(bh);
BUFFER_TRACE(bh, "return this buffer");
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index c839332be56b..d1ebb1d41d17 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -570,7 +570,6 @@ static void write_one_revoke_record(transaction_t *transaction,
int csum_size = 0;
struct buffer_head *descriptor;
int sz, offset;
- journal_header_t *header;
/* If we are already aborting, this all becomes a noop. We
still need to go round the loop in
@@ -600,13 +599,10 @@ static void write_one_revoke_record(transaction_t *transaction,
}
if (!descriptor) {
- descriptor = jbd2_journal_get_descriptor_buffer(journal);
+ descriptor = jbd2_journal_get_descriptor_buffer(transaction,
+ JBD2_REVOKE_BLOCK);
if (!descriptor)
return;
- header = (journal_header_t *)descriptor->b_data;
- header->h_magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
- header->h_blocktype = cpu_to_be32(JBD2_REVOKE_BLOCK);
- header->h_sequence = cpu_to_be32(transaction->t_tid);
/* Record it so that we can wait for IO completion later */
BUFFER_TRACE(descriptor, "file in log_bufs");
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 5716d9554e77..3649cb8d3a41 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1137,7 +1137,7 @@ static inline void jbd2_unfile_log_bh(struct buffer_head *bh)
}
/* Log buffer allocation */
-struct buffer_head *jbd2_journal_get_descriptor_buffer(journal_t *journal);
+struct buffer_head *jbd2_journal_get_descriptor_buffer(transaction_t *, int);
int jbd2_journal_next_log_block(journal_t *, unsigned long long *);
int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
unsigned long *block);
--
2.6.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling
2016-01-07 14:41 [PATCH 0/4] JBD2 cleanups Jan Kara
2016-01-07 14:41 ` [PATCH 1/4] jbd2: Remove some unnecessary arguments of jbd2_journal_write_revoke_records Jan Kara
2016-01-07 14:41 ` [PATCH 2/4] jbd2: Factor out common descriptor block initialization Jan Kara
@ 2016-01-07 14:41 ` Jan Kara
2016-01-07 17:16 ` Darrick J. Wong
2016-02-23 4:27 ` Theodore Ts'o
2016-01-07 14:41 ` [PATCH 4/4] jbd2: Save some atomic ops in __JI_COMMIT_RUNNING handling Jan Kara
3 siblings, 2 replies; 12+ messages in thread
From: Jan Kara @ 2016-01-07 14:41 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, Jan Kara
Revoke and tag descriptor blocks are just different kinds of descriptor
blocks and thus have checksum in the same place. Unify computation and
checking of checksums for these.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/commit.c | 18 +-----------------
fs/jbd2/journal.c | 15 +++++++++++++++
fs/jbd2/recovery.c | 31 +++++--------------------------
fs/jbd2/revoke.c | 19 ++-----------------
include/linux/jbd2.h | 8 ++------
5 files changed, 25 insertions(+), 66 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index cf221f3d955a..ae832cd44dd8 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -317,22 +317,6 @@ static void write_tag_block(journal_t *j, journal_block_tag_t *tag,
tag->t_blocknr_high = cpu_to_be32((block >> 31) >> 1);
}
-static void jbd2_descr_block_csum_set(journal_t *j,
- struct buffer_head *bh)
-{
- struct jbd2_journal_block_tail *tail;
- __u32 csum;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return;
-
- tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
- sizeof(struct jbd2_journal_block_tail));
- tail->t_checksum = 0;
- csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
- tail->t_checksum = cpu_to_be32(csum);
-}
-
static void jbd2_block_tag_csum_set(journal_t *j, journal_block_tag_t *tag,
struct buffer_head *bh, __u32 sequence)
{
@@ -714,7 +698,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
tag->t_flags |= cpu_to_be16(JBD2_FLAG_LAST_TAG);
- jbd2_descr_block_csum_set(journal, descriptor);
+ jbd2_descriptor_block_csum_set(journal, descriptor);
start_journal_io:
for (i = 0; i < bufs; i++) {
struct buffer_head *bh = wbuf[i];
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 28d05bd9a588..defa962a8e15 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -834,6 +834,21 @@ jbd2_journal_get_descriptor_buffer(transaction_t *transaction, int type)
return bh;
}
+void jbd2_descriptor_block_csum_set(journal_t *j, struct buffer_head *bh)
+{
+ struct jbd2_journal_block_tail *tail;
+ __u32 csum;
+
+ if (!jbd2_journal_has_csum_v2or3(j))
+ return;
+
+ tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
+ sizeof(struct jbd2_journal_block_tail));
+ tail->t_checksum = 0;
+ csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
+ tail->t_checksum = cpu_to_be32(csum);
+}
+
/*
* Return tid of the oldest transaction in the journal and block in the journal
* where the transaction starts.
diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 7f277e49fe88..08a456b96e4e 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -174,8 +174,7 @@ static int jread(struct buffer_head **bhp, journal_t *journal,
return 0;
}
-static int jbd2_descr_block_csum_verify(journal_t *j,
- void *buf)
+static int jbd2_descriptor_block_csum_verify(journal_t *j, void *buf)
{
struct jbd2_journal_block_tail *tail;
__be32 provided;
@@ -522,8 +521,8 @@ static int do_one_pass(journal_t *journal,
descr_csum_size =
sizeof(struct jbd2_journal_block_tail);
if (descr_csum_size > 0 &&
- !jbd2_descr_block_csum_verify(journal,
- bh->b_data)) {
+ !jbd2_descriptor_block_csum_verify(journal,
+ bh->b_data)) {
printk(KERN_ERR "JBD2: Invalid checksum "
"recovering block %lu in log\n",
next_log_block);
@@ -811,26 +810,6 @@ static int do_one_pass(journal_t *journal,
return err;
}
-static int jbd2_revoke_block_csum_verify(journal_t *j,
- void *buf)
-{
- struct jbd2_journal_revoke_tail *tail;
- __be32 provided;
- __u32 calculated;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return 1;
-
- tail = (struct jbd2_journal_revoke_tail *)(buf + j->j_blocksize -
- sizeof(struct jbd2_journal_revoke_tail));
- provided = tail->r_checksum;
- tail->r_checksum = 0;
- calculated = jbd2_chksum(j, j->j_csum_seed, buf, j->j_blocksize);
- tail->r_checksum = provided;
-
- return provided == cpu_to_be32(calculated);
-}
-
/* Scan a revoke record, marking all blocks mentioned as revoked. */
static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
@@ -846,11 +825,11 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
offset = sizeof(jbd2_journal_revoke_header_t);
rcount = be32_to_cpu(header->r_count);
- if (!jbd2_revoke_block_csum_verify(journal, header))
+ if (!jbd2_descriptor_block_csum_verify(journal, header))
return -EFSBADCRC;
if (jbd2_journal_has_csum_v2or3(journal))
- csum_size = sizeof(struct jbd2_journal_revoke_tail);
+ csum_size = sizeof(struct jbd2_journal_block_tail);
if (rcount > journal->j_blocksize - csum_size)
return -EINVAL;
max = rcount;
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index d1ebb1d41d17..91171dc352cb 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -583,7 +583,7 @@ static void write_one_revoke_record(transaction_t *transaction,
/* Do we need to leave space at the end for a checksum? */
if (jbd2_journal_has_csum_v2or3(journal))
- csum_size = sizeof(struct jbd2_journal_revoke_tail);
+ csum_size = sizeof(struct jbd2_journal_block_tail);
if (jbd2_has_feature_64bit(journal))
sz = 8;
@@ -623,21 +623,6 @@ static void write_one_revoke_record(transaction_t *transaction,
*offsetp = offset;
}
-static void jbd2_revoke_csum_set(journal_t *j, struct buffer_head *bh)
-{
- struct jbd2_journal_revoke_tail *tail;
- __u32 csum;
-
- if (!jbd2_journal_has_csum_v2or3(j))
- return;
-
- tail = (struct jbd2_journal_revoke_tail *)(bh->b_data + j->j_blocksize -
- sizeof(struct jbd2_journal_revoke_tail));
- tail->r_checksum = 0;
- csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
- tail->r_checksum = cpu_to_be32(csum);
-}
-
/*
* Flush a revoke descriptor out to the journal. If we are aborting,
* this is a noop; otherwise we are generating a buffer which needs to
@@ -658,7 +643,7 @@ static void flush_descriptor(journal_t *journal,
header = (jbd2_journal_revoke_header_t *)descriptor->b_data;
header->r_count = cpu_to_be32(offset);
- jbd2_revoke_csum_set(journal, descriptor);
+ jbd2_descriptor_block_csum_set(journal, descriptor);
set_buffer_jwrite(descriptor);
BUFFER_TRACE(descriptor, "write");
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 3649cb8d3a41..fd1083c46c61 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -200,7 +200,7 @@ typedef struct journal_block_tag_s
__be32 t_blocknr_high; /* most-significant high 32bits. */
} journal_block_tag_t;
-/* Tail of descriptor block, for checksumming */
+/* Tail of descriptor or revoke block, for checksumming */
struct jbd2_journal_block_tail {
__be32 t_checksum; /* crc32c(uuid+descr_block) */
};
@@ -215,11 +215,6 @@ typedef struct jbd2_journal_revoke_header_s
__be32 r_count; /* Count of bytes used in the block */
} jbd2_journal_revoke_header_t;
-/* Tail of revoke block, for checksumming */
-struct jbd2_journal_revoke_tail {
- __be32 r_checksum; /* crc32c(uuid+revoke_block) */
-};
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling
2016-01-07 14:41 ` [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling Jan Kara
@ 2016-01-07 17:16 ` Darrick J. Wong
2016-01-12 12:41 ` Jan Kara
2016-02-23 4:27 ` Theodore Ts'o
1 sibling, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2016-01-07 17:16 UTC (permalink / raw)
To: Jan Kara; +Cc: Ted Tso, linux-ext4
On Thu, Jan 07, 2016 at 03:41:54PM +0100, Jan Kara wrote:
> Revoke and tag descriptor blocks are just different kinds of descriptor
> blocks and thus have checksum in the same place. Unify computation and
> checking of checksums for these.
Looks ok to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Is there a corresponding update to e2fsprogs' journal code?
--D
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/jbd2/commit.c | 18 +-----------------
> fs/jbd2/journal.c | 15 +++++++++++++++
> fs/jbd2/recovery.c | 31 +++++--------------------------
> fs/jbd2/revoke.c | 19 ++-----------------
> include/linux/jbd2.h | 8 ++------
> 5 files changed, 25 insertions(+), 66 deletions(-)
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index cf221f3d955a..ae832cd44dd8 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -317,22 +317,6 @@ static void write_tag_block(journal_t *j, journal_block_tag_t *tag,
> tag->t_blocknr_high = cpu_to_be32((block >> 31) >> 1);
> }
>
> -static void jbd2_descr_block_csum_set(journal_t *j,
> - struct buffer_head *bh)
> -{
> - struct jbd2_journal_block_tail *tail;
> - __u32 csum;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return;
> -
> - tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> - sizeof(struct jbd2_journal_block_tail));
> - tail->t_checksum = 0;
> - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> - tail->t_checksum = cpu_to_be32(csum);
> -}
> -
> static void jbd2_block_tag_csum_set(journal_t *j, journal_block_tag_t *tag,
> struct buffer_head *bh, __u32 sequence)
> {
> @@ -714,7 +698,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
>
> tag->t_flags |= cpu_to_be16(JBD2_FLAG_LAST_TAG);
>
> - jbd2_descr_block_csum_set(journal, descriptor);
> + jbd2_descriptor_block_csum_set(journal, descriptor);
> start_journal_io:
> for (i = 0; i < bufs; i++) {
> struct buffer_head *bh = wbuf[i];
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 28d05bd9a588..defa962a8e15 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -834,6 +834,21 @@ jbd2_journal_get_descriptor_buffer(transaction_t *transaction, int type)
> return bh;
> }
>
> +void jbd2_descriptor_block_csum_set(journal_t *j, struct buffer_head *bh)
> +{
> + struct jbd2_journal_block_tail *tail;
> + __u32 csum;
> +
> + if (!jbd2_journal_has_csum_v2or3(j))
> + return;
> +
> + tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> + sizeof(struct jbd2_journal_block_tail));
> + tail->t_checksum = 0;
> + csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> + tail->t_checksum = cpu_to_be32(csum);
> +}
> +
> /*
> * Return tid of the oldest transaction in the journal and block in the journal
> * where the transaction starts.
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 7f277e49fe88..08a456b96e4e 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -174,8 +174,7 @@ static int jread(struct buffer_head **bhp, journal_t *journal,
> return 0;
> }
>
> -static int jbd2_descr_block_csum_verify(journal_t *j,
> - void *buf)
> +static int jbd2_descriptor_block_csum_verify(journal_t *j, void *buf)
> {
> struct jbd2_journal_block_tail *tail;
> __be32 provided;
> @@ -522,8 +521,8 @@ static int do_one_pass(journal_t *journal,
> descr_csum_size =
> sizeof(struct jbd2_journal_block_tail);
> if (descr_csum_size > 0 &&
> - !jbd2_descr_block_csum_verify(journal,
> - bh->b_data)) {
> + !jbd2_descriptor_block_csum_verify(journal,
> + bh->b_data)) {
> printk(KERN_ERR "JBD2: Invalid checksum "
> "recovering block %lu in log\n",
> next_log_block);
> @@ -811,26 +810,6 @@ static int do_one_pass(journal_t *journal,
> return err;
> }
>
> -static int jbd2_revoke_block_csum_verify(journal_t *j,
> - void *buf)
> -{
> - struct jbd2_journal_revoke_tail *tail;
> - __be32 provided;
> - __u32 calculated;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return 1;
> -
> - tail = (struct jbd2_journal_revoke_tail *)(buf + j->j_blocksize -
> - sizeof(struct jbd2_journal_revoke_tail));
> - provided = tail->r_checksum;
> - tail->r_checksum = 0;
> - calculated = jbd2_chksum(j, j->j_csum_seed, buf, j->j_blocksize);
> - tail->r_checksum = provided;
> -
> - return provided == cpu_to_be32(calculated);
> -}
> -
> /* Scan a revoke record, marking all blocks mentioned as revoked. */
>
> static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> @@ -846,11 +825,11 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> offset = sizeof(jbd2_journal_revoke_header_t);
> rcount = be32_to_cpu(header->r_count);
>
> - if (!jbd2_revoke_block_csum_verify(journal, header))
> + if (!jbd2_descriptor_block_csum_verify(journal, header))
> return -EFSBADCRC;
>
> if (jbd2_journal_has_csum_v2or3(journal))
> - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> + csum_size = sizeof(struct jbd2_journal_block_tail);
> if (rcount > journal->j_blocksize - csum_size)
> return -EINVAL;
> max = rcount;
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index d1ebb1d41d17..91171dc352cb 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -583,7 +583,7 @@ static void write_one_revoke_record(transaction_t *transaction,
>
> /* Do we need to leave space at the end for a checksum? */
> if (jbd2_journal_has_csum_v2or3(journal))
> - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> + csum_size = sizeof(struct jbd2_journal_block_tail);
>
> if (jbd2_has_feature_64bit(journal))
> sz = 8;
> @@ -623,21 +623,6 @@ static void write_one_revoke_record(transaction_t *transaction,
> *offsetp = offset;
> }
>
> -static void jbd2_revoke_csum_set(journal_t *j, struct buffer_head *bh)
> -{
> - struct jbd2_journal_revoke_tail *tail;
> - __u32 csum;
> -
> - if (!jbd2_journal_has_csum_v2or3(j))
> - return;
> -
> - tail = (struct jbd2_journal_revoke_tail *)(bh->b_data + j->j_blocksize -
> - sizeof(struct jbd2_journal_revoke_tail));
> - tail->r_checksum = 0;
> - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> - tail->r_checksum = cpu_to_be32(csum);
> -}
> -
> /*
> * Flush a revoke descriptor out to the journal. If we are aborting,
> * this is a noop; otherwise we are generating a buffer which needs to
> @@ -658,7 +643,7 @@ static void flush_descriptor(journal_t *journal,
>
> header = (jbd2_journal_revoke_header_t *)descriptor->b_data;
> header->r_count = cpu_to_be32(offset);
> - jbd2_revoke_csum_set(journal, descriptor);
> + jbd2_descriptor_block_csum_set(journal, descriptor);
>
> set_buffer_jwrite(descriptor);
> BUFFER_TRACE(descriptor, "write");
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 3649cb8d3a41..fd1083c46c61 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -200,7 +200,7 @@ typedef struct journal_block_tag_s
> __be32 t_blocknr_high; /* most-significant high 32bits. */
> } journal_block_tag_t;
>
> -/* Tail of descriptor block, for checksumming */
> +/* Tail of descriptor or revoke block, for checksumming */
> struct jbd2_journal_block_tail {
> __be32 t_checksum; /* crc32c(uuid+descr_block) */
> };
> @@ -215,11 +215,6 @@ typedef struct jbd2_journal_revoke_header_s
> __be32 r_count; /* Count of bytes used in the block */
> } jbd2_journal_revoke_header_t;
>
> -/* Tail of revoke block, for checksumming */
> -struct jbd2_journal_revoke_tail {
> - __be32 r_checksum; /* crc32c(uuid+revoke_block) */
> -};
> -
> /* Definitions for the journal tag flags word: */
> #define JBD2_FLAG_ESCAPE 1 /* on-disk block is escaped */
> #define JBD2_FLAG_SAME_UUID 2 /* block has same uuid as previous */
> @@ -1138,6 +1133,7 @@ static inline void jbd2_unfile_log_bh(struct buffer_head *bh)
>
> /* Log buffer allocation */
> struct buffer_head *jbd2_journal_get_descriptor_buffer(transaction_t *, int);
> +void jbd2_descriptor_block_csum_set(journal_t *, struct buffer_head *);
> int jbd2_journal_next_log_block(journal_t *, unsigned long long *);
> int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
> unsigned long *block);
> --
> 2.6.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling
2016-01-07 17:16 ` Darrick J. Wong
@ 2016-01-12 12:41 ` Jan Kara
2016-01-13 1:25 ` Darrick J. Wong
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2016-01-12 12:41 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Jan Kara, Ted Tso, linux-ext4
On Thu 07-01-16 09:16:45, Darrick J. Wong wrote:
> On Thu, Jan 07, 2016 at 03:41:54PM +0100, Jan Kara wrote:
> > Revoke and tag descriptor blocks are just different kinds of descriptor
> > blocks and thus have checksum in the same place. Unify computation and
> > checking of checksums for these.
>
> Looks ok to me,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Thanks for review.
> Is there a corresponding update to e2fsprogs' journal code?
No, I can write one if Ted decides to merge this in JBD2.
Honza
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/jbd2/commit.c | 18 +-----------------
> > fs/jbd2/journal.c | 15 +++++++++++++++
> > fs/jbd2/recovery.c | 31 +++++--------------------------
> > fs/jbd2/revoke.c | 19 ++-----------------
> > include/linux/jbd2.h | 8 ++------
> > 5 files changed, 25 insertions(+), 66 deletions(-)
> >
> > diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> > index cf221f3d955a..ae832cd44dd8 100644
> > --- a/fs/jbd2/commit.c
> > +++ b/fs/jbd2/commit.c
> > @@ -317,22 +317,6 @@ static void write_tag_block(journal_t *j, journal_block_tag_t *tag,
> > tag->t_blocknr_high = cpu_to_be32((block >> 31) >> 1);
> > }
> >
> > -static void jbd2_descr_block_csum_set(journal_t *j,
> > - struct buffer_head *bh)
> > -{
> > - struct jbd2_journal_block_tail *tail;
> > - __u32 csum;
> > -
> > - if (!jbd2_journal_has_csum_v2or3(j))
> > - return;
> > -
> > - tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> > - sizeof(struct jbd2_journal_block_tail));
> > - tail->t_checksum = 0;
> > - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > - tail->t_checksum = cpu_to_be32(csum);
> > -}
> > -
> > static void jbd2_block_tag_csum_set(journal_t *j, journal_block_tag_t *tag,
> > struct buffer_head *bh, __u32 sequence)
> > {
> > @@ -714,7 +698,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> >
> > tag->t_flags |= cpu_to_be16(JBD2_FLAG_LAST_TAG);
> >
> > - jbd2_descr_block_csum_set(journal, descriptor);
> > + jbd2_descriptor_block_csum_set(journal, descriptor);
> > start_journal_io:
> > for (i = 0; i < bufs; i++) {
> > struct buffer_head *bh = wbuf[i];
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index 28d05bd9a588..defa962a8e15 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -834,6 +834,21 @@ jbd2_journal_get_descriptor_buffer(transaction_t *transaction, int type)
> > return bh;
> > }
> >
> > +void jbd2_descriptor_block_csum_set(journal_t *j, struct buffer_head *bh)
> > +{
> > + struct jbd2_journal_block_tail *tail;
> > + __u32 csum;
> > +
> > + if (!jbd2_journal_has_csum_v2or3(j))
> > + return;
> > +
> > + tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> > + sizeof(struct jbd2_journal_block_tail));
> > + tail->t_checksum = 0;
> > + csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > + tail->t_checksum = cpu_to_be32(csum);
> > +}
> > +
> > /*
> > * Return tid of the oldest transaction in the journal and block in the journal
> > * where the transaction starts.
> > diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> > index 7f277e49fe88..08a456b96e4e 100644
> > --- a/fs/jbd2/recovery.c
> > +++ b/fs/jbd2/recovery.c
> > @@ -174,8 +174,7 @@ static int jread(struct buffer_head **bhp, journal_t *journal,
> > return 0;
> > }
> >
> > -static int jbd2_descr_block_csum_verify(journal_t *j,
> > - void *buf)
> > +static int jbd2_descriptor_block_csum_verify(journal_t *j, void *buf)
> > {
> > struct jbd2_journal_block_tail *tail;
> > __be32 provided;
> > @@ -522,8 +521,8 @@ static int do_one_pass(journal_t *journal,
> > descr_csum_size =
> > sizeof(struct jbd2_journal_block_tail);
> > if (descr_csum_size > 0 &&
> > - !jbd2_descr_block_csum_verify(journal,
> > - bh->b_data)) {
> > + !jbd2_descriptor_block_csum_verify(journal,
> > + bh->b_data)) {
> > printk(KERN_ERR "JBD2: Invalid checksum "
> > "recovering block %lu in log\n",
> > next_log_block);
> > @@ -811,26 +810,6 @@ static int do_one_pass(journal_t *journal,
> > return err;
> > }
> >
> > -static int jbd2_revoke_block_csum_verify(journal_t *j,
> > - void *buf)
> > -{
> > - struct jbd2_journal_revoke_tail *tail;
> > - __be32 provided;
> > - __u32 calculated;
> > -
> > - if (!jbd2_journal_has_csum_v2or3(j))
> > - return 1;
> > -
> > - tail = (struct jbd2_journal_revoke_tail *)(buf + j->j_blocksize -
> > - sizeof(struct jbd2_journal_revoke_tail));
> > - provided = tail->r_checksum;
> > - tail->r_checksum = 0;
> > - calculated = jbd2_chksum(j, j->j_csum_seed, buf, j->j_blocksize);
> > - tail->r_checksum = provided;
> > -
> > - return provided == cpu_to_be32(calculated);
> > -}
> > -
> > /* Scan a revoke record, marking all blocks mentioned as revoked. */
> >
> > static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> > @@ -846,11 +825,11 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> > offset = sizeof(jbd2_journal_revoke_header_t);
> > rcount = be32_to_cpu(header->r_count);
> >
> > - if (!jbd2_revoke_block_csum_verify(journal, header))
> > + if (!jbd2_descriptor_block_csum_verify(journal, header))
> > return -EFSBADCRC;
> >
> > if (jbd2_journal_has_csum_v2or3(journal))
> > - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> > + csum_size = sizeof(struct jbd2_journal_block_tail);
> > if (rcount > journal->j_blocksize - csum_size)
> > return -EINVAL;
> > max = rcount;
> > diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> > index d1ebb1d41d17..91171dc352cb 100644
> > --- a/fs/jbd2/revoke.c
> > +++ b/fs/jbd2/revoke.c
> > @@ -583,7 +583,7 @@ static void write_one_revoke_record(transaction_t *transaction,
> >
> > /* Do we need to leave space at the end for a checksum? */
> > if (jbd2_journal_has_csum_v2or3(journal))
> > - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> > + csum_size = sizeof(struct jbd2_journal_block_tail);
> >
> > if (jbd2_has_feature_64bit(journal))
> > sz = 8;
> > @@ -623,21 +623,6 @@ static void write_one_revoke_record(transaction_t *transaction,
> > *offsetp = offset;
> > }
> >
> > -static void jbd2_revoke_csum_set(journal_t *j, struct buffer_head *bh)
> > -{
> > - struct jbd2_journal_revoke_tail *tail;
> > - __u32 csum;
> > -
> > - if (!jbd2_journal_has_csum_v2or3(j))
> > - return;
> > -
> > - tail = (struct jbd2_journal_revoke_tail *)(bh->b_data + j->j_blocksize -
> > - sizeof(struct jbd2_journal_revoke_tail));
> > - tail->r_checksum = 0;
> > - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > - tail->r_checksum = cpu_to_be32(csum);
> > -}
> > -
> > /*
> > * Flush a revoke descriptor out to the journal. If we are aborting,
> > * this is a noop; otherwise we are generating a buffer which needs to
> > @@ -658,7 +643,7 @@ static void flush_descriptor(journal_t *journal,
> >
> > header = (jbd2_journal_revoke_header_t *)descriptor->b_data;
> > header->r_count = cpu_to_be32(offset);
> > - jbd2_revoke_csum_set(journal, descriptor);
> > + jbd2_descriptor_block_csum_set(journal, descriptor);
> >
> > set_buffer_jwrite(descriptor);
> > BUFFER_TRACE(descriptor, "write");
> > diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> > index 3649cb8d3a41..fd1083c46c61 100644
> > --- a/include/linux/jbd2.h
> > +++ b/include/linux/jbd2.h
> > @@ -200,7 +200,7 @@ typedef struct journal_block_tag_s
> > __be32 t_blocknr_high; /* most-significant high 32bits. */
> > } journal_block_tag_t;
> >
> > -/* Tail of descriptor block, for checksumming */
> > +/* Tail of descriptor or revoke block, for checksumming */
> > struct jbd2_journal_block_tail {
> > __be32 t_checksum; /* crc32c(uuid+descr_block) */
> > };
> > @@ -215,11 +215,6 @@ typedef struct jbd2_journal_revoke_header_s
> > __be32 r_count; /* Count of bytes used in the block */
> > } jbd2_journal_revoke_header_t;
> >
> > -/* Tail of revoke block, for checksumming */
> > -struct jbd2_journal_revoke_tail {
> > - __be32 r_checksum; /* crc32c(uuid+revoke_block) */
> > -};
> > -
> > /* Definitions for the journal tag flags word: */
> > #define JBD2_FLAG_ESCAPE 1 /* on-disk block is escaped */
> > #define JBD2_FLAG_SAME_UUID 2 /* block has same uuid as previous */
> > @@ -1138,6 +1133,7 @@ static inline void jbd2_unfile_log_bh(struct buffer_head *bh)
> >
> > /* Log buffer allocation */
> > struct buffer_head *jbd2_journal_get_descriptor_buffer(transaction_t *, int);
> > +void jbd2_descriptor_block_csum_set(journal_t *, struct buffer_head *);
> > int jbd2_journal_next_log_block(journal_t *, unsigned long long *);
> > int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
> > unsigned long *block);
> > --
> > 2.6.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling
2016-01-12 12:41 ` Jan Kara
@ 2016-01-13 1:25 ` Darrick J. Wong
0 siblings, 0 replies; 12+ messages in thread
From: Darrick J. Wong @ 2016-01-13 1:25 UTC (permalink / raw)
To: Jan Kara; +Cc: Ted Tso, linux-ext4
On Tue, Jan 12, 2016 at 01:41:01PM +0100, Jan Kara wrote:
> On Thu 07-01-16 09:16:45, Darrick J. Wong wrote:
> > On Thu, Jan 07, 2016 at 03:41:54PM +0100, Jan Kara wrote:
> > > Revoke and tag descriptor blocks are just different kinds of descriptor
> > > blocks and thus have checksum in the same place. Unify computation and
> > > checking of checksums for these.
> >
> > Looks ok to me,
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Thanks for review.
>
> > Is there a corresponding update to e2fsprogs' journal code?
>
> No, I can write one if Ted decides to merge this in JBD2.
Ok, just checking. I'll review one if/when I see it. :)
--D
>
> Honza
>
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > > ---
> > > fs/jbd2/commit.c | 18 +-----------------
> > > fs/jbd2/journal.c | 15 +++++++++++++++
> > > fs/jbd2/recovery.c | 31 +++++--------------------------
> > > fs/jbd2/revoke.c | 19 ++-----------------
> > > include/linux/jbd2.h | 8 ++------
> > > 5 files changed, 25 insertions(+), 66 deletions(-)
> > >
> > > diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> > > index cf221f3d955a..ae832cd44dd8 100644
> > > --- a/fs/jbd2/commit.c
> > > +++ b/fs/jbd2/commit.c
> > > @@ -317,22 +317,6 @@ static void write_tag_block(journal_t *j, journal_block_tag_t *tag,
> > > tag->t_blocknr_high = cpu_to_be32((block >> 31) >> 1);
> > > }
> > >
> > > -static void jbd2_descr_block_csum_set(journal_t *j,
> > > - struct buffer_head *bh)
> > > -{
> > > - struct jbd2_journal_block_tail *tail;
> > > - __u32 csum;
> > > -
> > > - if (!jbd2_journal_has_csum_v2or3(j))
> > > - return;
> > > -
> > > - tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> > > - sizeof(struct jbd2_journal_block_tail));
> > > - tail->t_checksum = 0;
> > > - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > > - tail->t_checksum = cpu_to_be32(csum);
> > > -}
> > > -
> > > static void jbd2_block_tag_csum_set(journal_t *j, journal_block_tag_t *tag,
> > > struct buffer_head *bh, __u32 sequence)
> > > {
> > > @@ -714,7 +698,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> > >
> > > tag->t_flags |= cpu_to_be16(JBD2_FLAG_LAST_TAG);
> > >
> > > - jbd2_descr_block_csum_set(journal, descriptor);
> > > + jbd2_descriptor_block_csum_set(journal, descriptor);
> > > start_journal_io:
> > > for (i = 0; i < bufs; i++) {
> > > struct buffer_head *bh = wbuf[i];
> > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > > index 28d05bd9a588..defa962a8e15 100644
> > > --- a/fs/jbd2/journal.c
> > > +++ b/fs/jbd2/journal.c
> > > @@ -834,6 +834,21 @@ jbd2_journal_get_descriptor_buffer(transaction_t *transaction, int type)
> > > return bh;
> > > }
> > >
> > > +void jbd2_descriptor_block_csum_set(journal_t *j, struct buffer_head *bh)
> > > +{
> > > + struct jbd2_journal_block_tail *tail;
> > > + __u32 csum;
> > > +
> > > + if (!jbd2_journal_has_csum_v2or3(j))
> > > + return;
> > > +
> > > + tail = (struct jbd2_journal_block_tail *)(bh->b_data + j->j_blocksize -
> > > + sizeof(struct jbd2_journal_block_tail));
> > > + tail->t_checksum = 0;
> > > + csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > > + tail->t_checksum = cpu_to_be32(csum);
> > > +}
> > > +
> > > /*
> > > * Return tid of the oldest transaction in the journal and block in the journal
> > > * where the transaction starts.
> > > diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> > > index 7f277e49fe88..08a456b96e4e 100644
> > > --- a/fs/jbd2/recovery.c
> > > +++ b/fs/jbd2/recovery.c
> > > @@ -174,8 +174,7 @@ static int jread(struct buffer_head **bhp, journal_t *journal,
> > > return 0;
> > > }
> > >
> > > -static int jbd2_descr_block_csum_verify(journal_t *j,
> > > - void *buf)
> > > +static int jbd2_descriptor_block_csum_verify(journal_t *j, void *buf)
> > > {
> > > struct jbd2_journal_block_tail *tail;
> > > __be32 provided;
> > > @@ -522,8 +521,8 @@ static int do_one_pass(journal_t *journal,
> > > descr_csum_size =
> > > sizeof(struct jbd2_journal_block_tail);
> > > if (descr_csum_size > 0 &&
> > > - !jbd2_descr_block_csum_verify(journal,
> > > - bh->b_data)) {
> > > + !jbd2_descriptor_block_csum_verify(journal,
> > > + bh->b_data)) {
> > > printk(KERN_ERR "JBD2: Invalid checksum "
> > > "recovering block %lu in log\n",
> > > next_log_block);
> > > @@ -811,26 +810,6 @@ static int do_one_pass(journal_t *journal,
> > > return err;
> > > }
> > >
> > > -static int jbd2_revoke_block_csum_verify(journal_t *j,
> > > - void *buf)
> > > -{
> > > - struct jbd2_journal_revoke_tail *tail;
> > > - __be32 provided;
> > > - __u32 calculated;
> > > -
> > > - if (!jbd2_journal_has_csum_v2or3(j))
> > > - return 1;
> > > -
> > > - tail = (struct jbd2_journal_revoke_tail *)(buf + j->j_blocksize -
> > > - sizeof(struct jbd2_journal_revoke_tail));
> > > - provided = tail->r_checksum;
> > > - tail->r_checksum = 0;
> > > - calculated = jbd2_chksum(j, j->j_csum_seed, buf, j->j_blocksize);
> > > - tail->r_checksum = provided;
> > > -
> > > - return provided == cpu_to_be32(calculated);
> > > -}
> > > -
> > > /* Scan a revoke record, marking all blocks mentioned as revoked. */
> > >
> > > static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> > > @@ -846,11 +825,11 @@ static int scan_revoke_records(journal_t *journal, struct buffer_head *bh,
> > > offset = sizeof(jbd2_journal_revoke_header_t);
> > > rcount = be32_to_cpu(header->r_count);
> > >
> > > - if (!jbd2_revoke_block_csum_verify(journal, header))
> > > + if (!jbd2_descriptor_block_csum_verify(journal, header))
> > > return -EFSBADCRC;
> > >
> > > if (jbd2_journal_has_csum_v2or3(journal))
> > > - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> > > + csum_size = sizeof(struct jbd2_journal_block_tail);
> > > if (rcount > journal->j_blocksize - csum_size)
> > > return -EINVAL;
> > > max = rcount;
> > > diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> > > index d1ebb1d41d17..91171dc352cb 100644
> > > --- a/fs/jbd2/revoke.c
> > > +++ b/fs/jbd2/revoke.c
> > > @@ -583,7 +583,7 @@ static void write_one_revoke_record(transaction_t *transaction,
> > >
> > > /* Do we need to leave space at the end for a checksum? */
> > > if (jbd2_journal_has_csum_v2or3(journal))
> > > - csum_size = sizeof(struct jbd2_journal_revoke_tail);
> > > + csum_size = sizeof(struct jbd2_journal_block_tail);
> > >
> > > if (jbd2_has_feature_64bit(journal))
> > > sz = 8;
> > > @@ -623,21 +623,6 @@ static void write_one_revoke_record(transaction_t *transaction,
> > > *offsetp = offset;
> > > }
> > >
> > > -static void jbd2_revoke_csum_set(journal_t *j, struct buffer_head *bh)
> > > -{
> > > - struct jbd2_journal_revoke_tail *tail;
> > > - __u32 csum;
> > > -
> > > - if (!jbd2_journal_has_csum_v2or3(j))
> > > - return;
> > > -
> > > - tail = (struct jbd2_journal_revoke_tail *)(bh->b_data + j->j_blocksize -
> > > - sizeof(struct jbd2_journal_revoke_tail));
> > > - tail->r_checksum = 0;
> > > - csum = jbd2_chksum(j, j->j_csum_seed, bh->b_data, j->j_blocksize);
> > > - tail->r_checksum = cpu_to_be32(csum);
> > > -}
> > > -
> > > /*
> > > * Flush a revoke descriptor out to the journal. If we are aborting,
> > > * this is a noop; otherwise we are generating a buffer which needs to
> > > @@ -658,7 +643,7 @@ static void flush_descriptor(journal_t *journal,
> > >
> > > header = (jbd2_journal_revoke_header_t *)descriptor->b_data;
> > > header->r_count = cpu_to_be32(offset);
> > > - jbd2_revoke_csum_set(journal, descriptor);
> > > + jbd2_descriptor_block_csum_set(journal, descriptor);
> > >
> > > set_buffer_jwrite(descriptor);
> > > BUFFER_TRACE(descriptor, "write");
> > > diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> > > index 3649cb8d3a41..fd1083c46c61 100644
> > > --- a/include/linux/jbd2.h
> > > +++ b/include/linux/jbd2.h
> > > @@ -200,7 +200,7 @@ typedef struct journal_block_tag_s
> > > __be32 t_blocknr_high; /* most-significant high 32bits. */
> > > } journal_block_tag_t;
> > >
> > > -/* Tail of descriptor block, for checksumming */
> > > +/* Tail of descriptor or revoke block, for checksumming */
> > > struct jbd2_journal_block_tail {
> > > __be32 t_checksum; /* crc32c(uuid+descr_block) */
> > > };
> > > @@ -215,11 +215,6 @@ typedef struct jbd2_journal_revoke_header_s
> > > __be32 r_count; /* Count of bytes used in the block */
> > > } jbd2_journal_revoke_header_t;
> > >
> > > -/* Tail of revoke block, for checksumming */
> > > -struct jbd2_journal_revoke_tail {
> > > - __be32 r_checksum; /* crc32c(uuid+revoke_block) */
> > > -};
> > > -
> > > /* Definitions for the journal tag flags word: */
> > > #define JBD2_FLAG_ESCAPE 1 /* on-disk block is escaped */
> > > #define JBD2_FLAG_SAME_UUID 2 /* block has same uuid as previous */
> > > @@ -1138,6 +1133,7 @@ static inline void jbd2_unfile_log_bh(struct buffer_head *bh)
> > >
> > > /* Log buffer allocation */
> > > struct buffer_head *jbd2_journal_get_descriptor_buffer(transaction_t *, int);
> > > +void jbd2_descriptor_block_csum_set(journal_t *, struct buffer_head *);
> > > int jbd2_journal_next_log_block(journal_t *, unsigned long long *);
> > > int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
> > > unsigned long *block);
> > > --
> > > 2.6.2
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling
2016-01-07 14:41 ` [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling Jan Kara
2016-01-07 17:16 ` Darrick J. Wong
@ 2016-02-23 4:27 ` Theodore Ts'o
1 sibling, 0 replies; 12+ messages in thread
From: Theodore Ts'o @ 2016-02-23 4:27 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-ext4
On Thu, Jan 07, 2016 at 03:41:54PM +0100, Jan Kara wrote:
> Revoke and tag descriptor blocks are just different kinds of descriptor
> blocks and thus have checksum in the same place. Unify computation and
> checking of checksums for these.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] jbd2: Save some atomic ops in __JI_COMMIT_RUNNING handling
2016-01-07 14:41 [PATCH 0/4] JBD2 cleanups Jan Kara
` (2 preceding siblings ...)
2016-01-07 14:41 ` [PATCH 3/4] jbd2: Unify revoke and tag block checksum handling Jan Kara
@ 2016-01-07 14:41 ` Jan Kara
2016-02-23 4:27 ` Theodore Ts'o
3 siblings, 1 reply; 12+ messages in thread
From: Jan Kara @ 2016-01-07 14:41 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, Jan Kara
Currently we used atomic bit operations to manipulate
__JI_COMMIT_RUNNING bit. However this is unnecessary as i_flags are
always written and read under j_list_lock. So just change the operations
to standard bit operations.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/jbd2/commit.c | 12 ++++++------
fs/jbd2/journal.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index ae832cd44dd8..517f2de784cf 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -220,7 +220,7 @@ static int journal_submit_data_buffers(journal_t *journal,
spin_lock(&journal->j_list_lock);
list_for_each_entry(jinode, &commit_transaction->t_inode_list, i_list) {
mapping = jinode->i_vfs_inode->i_mapping;
- set_bit(__JI_COMMIT_RUNNING, &jinode->i_flags);
+ jinode->i_flags |= JI_COMMIT_RUNNING;
spin_unlock(&journal->j_list_lock);
/*
* submit the inode data buffers. We use writepage
@@ -234,8 +234,8 @@ static int journal_submit_data_buffers(journal_t *journal,
ret = err;
spin_lock(&journal->j_list_lock);
J_ASSERT(jinode->i_transaction == commit_transaction);
- clear_bit(__JI_COMMIT_RUNNING, &jinode->i_flags);
- smp_mb__after_atomic();
+ jinode->i_flags &= ~JI_COMMIT_RUNNING;
+ smp_mb();
wake_up_bit(&jinode->i_flags, __JI_COMMIT_RUNNING);
}
spin_unlock(&journal->j_list_lock);
@@ -256,7 +256,7 @@ static int journal_finish_inode_data_buffers(journal_t *journal,
/* For locking, see the comment in journal_submit_data_buffers() */
spin_lock(&journal->j_list_lock);
list_for_each_entry(jinode, &commit_transaction->t_inode_list, i_list) {
- set_bit(__JI_COMMIT_RUNNING, &jinode->i_flags);
+ jinode->i_flags |= JI_COMMIT_RUNNING;
spin_unlock(&journal->j_list_lock);
err = filemap_fdatawait(jinode->i_vfs_inode->i_mapping);
if (err) {
@@ -272,8 +272,8 @@ static int journal_finish_inode_data_buffers(journal_t *journal,
ret = err;
}
spin_lock(&journal->j_list_lock);
- clear_bit(__JI_COMMIT_RUNNING, &jinode->i_flags);
- smp_mb__after_atomic();
+ jinode->i_flags &= ~JI_COMMIT_RUNNING;
+ smp_mb();
wake_up_bit(&jinode->i_flags, __JI_COMMIT_RUNNING);
}
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index defa962a8e15..7bf1683e81af 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2587,7 +2587,7 @@ void jbd2_journal_release_jbd_inode(journal_t *journal,
restart:
spin_lock(&journal->j_list_lock);
/* Is commit writing out inode - we have to wait */
- if (test_bit(__JI_COMMIT_RUNNING, &jinode->i_flags)) {
+ if (jinode->i_flags & JI_COMMIT_RUNNING) {
wait_queue_head_t *wq;
DEFINE_WAIT_BIT(wait, &jinode->i_flags, __JI_COMMIT_RUNNING);
wq = bit_waitqueue(&jinode->i_flags, __JI_COMMIT_RUNNING);
--
2.6.2
^ permalink raw reply related [flat|nested] 12+ messages in thread