| Message ID | 1321695269-3319-1-git-send-email-xiaoqiangnk@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sat 19-11-11 17:34:29, Yongqiang Yang wrote: > Currently, we clear revoked flag only when a block is reused. However, > this can tigger a false journal error. Consider a situation when a block > is used as a meta block and is deleted(revoked) in ordered mode, then the > block is allocated as a data block to a file. At this moment, user changes > the file's journal mode from ordered to journaled and truncates the file. > The block will be considered re-revoked by journal because it has revoked > flag still pending from the last transaction and an assertion triggers. > > We fix the problem by keeping the revoked status more uptodate - we clear > revoked flag when switching revoke tables to reflect there is no revoked > buffers in current transaction any more. Thanks. Merged into my tree. Will you create the same patch for JBD2 as well? Honza > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> > --- > fs/jbd/commit.c | 6 ++++++ > fs/jbd/revoke.c | 34 ++++++++++++++++++++++++++++++++++ > include/linux/jbd.h | 1 + > 3 files changed, 41 insertions(+), 0 deletions(-) > > diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c > index 8799207..f2b9a57 100644 > --- a/fs/jbd/commit.c > +++ b/fs/jbd/commit.c > @@ -392,6 +392,12 @@ void journal_commit_transaction(journal_t *journal) > jbd_debug (3, "JBD: commit phase 1\n"); > > /* > + * Clear revoked flag to reflect there is no revoked buffers > + * in the next transaction which is going to be started. > + */ > + journal_clear_buffer_revoked_flags(journal); > + > + /* > * Switch to a new revoke table. > */ > journal_switch_revoke_table(journal); > diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c > index 034eb82..501d363 100644 > --- a/fs/jbd/revoke.c > +++ b/fs/jbd/revoke.c > @@ -47,6 +47,10 @@ > * overwriting the new data. We don't even need to clear the revoke > * bit here. > * > + * We cache revoke status of a buffer in the current transaction in b_states > + * bits. As the name says, revokevalid flag indicates that the cached revoke > + * status of a buffer is valid and we can rely on the cached status. > + * > * Revoke information on buffers is a tri-state value: > * > * RevokeValid clear: no cached revoke status, need to look it up > @@ -474,6 +478,36 @@ int journal_cancel_revoke(handle_t *handle, struct journal_head *jh) > return did_revoke; > } > > +/* > + * journal_clear_revoked_flag clears revoked flag of buffers in > + * revoke table to reflect there is no revoked buffers in the next > + * transaction which is going to be started. > + */ > +void journal_clear_buffer_revoked_flags(journal_t *journal) > +{ > + struct jbd_revoke_table_s *revoke = journal->j_revoke; > + int i = 0; > + > + for (i = 0; i < revoke->hash_size; i++) { > + struct list_head *hash_list; > + struct list_head *list_entry; > + hash_list = &revoke->hash_table[i]; > + > + list_for_each(list_entry, hash_list) { > + struct jbd_revoke_record_s *record; > + struct buffer_head *bh; > + record = (struct jbd_revoke_record_s *)list_entry; > + bh = __find_get_block(journal->j_fs_dev, > + record->blocknr, > + journal->j_blocksize); > + if (bh) { > + clear_buffer_revoked(bh); > + __brelse(bh); > + } > + } > + } > +} > + > /* journal_switch_revoke table select j_revoke for next transaction > * we do not want to suspend any processing until all revokes are > * written -bzzz > diff --git a/include/linux/jbd.h b/include/linux/jbd.h > index 0f9f0b6..5aa964e 100644 > --- a/include/linux/jbd.h > +++ b/include/linux/jbd.h > @@ -918,6 +918,7 @@ extern int journal_set_revoke(journal_t *, unsigned int, tid_t); > extern int journal_test_revoke(journal_t *, unsigned int, tid_t); > extern void journal_clear_revoke(journal_t *); > extern void journal_switch_revoke_table(journal_t *journal); > +extern void journal_clear_buffer_revoked_flags(journal_t *journal); > > /* > * The log thread user interface: > -- > 1.7.5.1 >
On Tue, Nov 22, 2011 at 8:23 AM, Jan Kara <jack@suse.cz> wrote: > On Sat 19-11-11 17:34:29, Yongqiang Yang wrote: >> Currently, we clear revoked flag only when a block is reused. However, >> this can tigger a false journal error. Consider a situation when a block >> is used as a meta block and is deleted(revoked) in ordered mode, then the >> block is allocated as a data block to a file. At this moment, user changes >> the file's journal mode from ordered to journaled and truncates the file. >> The block will be considered re-revoked by journal because it has revoked >> flag still pending from the last transaction and an assertion triggers. >> >> We fix the problem by keeping the revoked status more uptodate - we clear >> revoked flag when switching revoke tables to reflect there is no revoked >> buffers in current transaction any more. > Thanks. Merged into my tree. Will you create the same patch for JBD2 as > well? YES! I had sent out the patch for jbd2. It seems that Ted is busy these days. Yongqiang. > > Honza > >> >> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> >> --- >> fs/jbd/commit.c | 6 ++++++ >> fs/jbd/revoke.c | 34 ++++++++++++++++++++++++++++++++++ >> include/linux/jbd.h | 1 + >> 3 files changed, 41 insertions(+), 0 deletions(-) >> >> diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c >> index 8799207..f2b9a57 100644 >> --- a/fs/jbd/commit.c >> +++ b/fs/jbd/commit.c >> @@ -392,6 +392,12 @@ void journal_commit_transaction(journal_t *journal) >> jbd_debug (3, "JBD: commit phase 1\n"); >> >> /* >> + * Clear revoked flag to reflect there is no revoked buffers >> + * in the next transaction which is going to be started. >> + */ >> + journal_clear_buffer_revoked_flags(journal); >> + >> + /* >> * Switch to a new revoke table. >> */ >> journal_switch_revoke_table(journal); >> diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c >> index 034eb82..501d363 100644 >> --- a/fs/jbd/revoke.c >> +++ b/fs/jbd/revoke.c >> @@ -47,6 +47,10 @@ >> * overwriting the new data. We don't even need to clear the revoke >> * bit here. >> * >> + * We cache revoke status of a buffer in the current transaction in b_states >> + * bits. As the name says, revokevalid flag indicates that the cached revoke >> + * status of a buffer is valid and we can rely on the cached status. >> + * >> * Revoke information on buffers is a tri-state value: >> * >> * RevokeValid clear: no cached revoke status, need to look it up >> @@ -474,6 +478,36 @@ int journal_cancel_revoke(handle_t *handle, struct journal_head *jh) >> return did_revoke; >> } >> >> +/* >> + * journal_clear_revoked_flag clears revoked flag of buffers in >> + * revoke table to reflect there is no revoked buffers in the next >> + * transaction which is going to be started. >> + */ >> +void journal_clear_buffer_revoked_flags(journal_t *journal) >> +{ >> + struct jbd_revoke_table_s *revoke = journal->j_revoke; >> + int i = 0; >> + >> + for (i = 0; i < revoke->hash_size; i++) { >> + struct list_head *hash_list; >> + struct list_head *list_entry; >> + hash_list = &revoke->hash_table[i]; >> + >> + list_for_each(list_entry, hash_list) { >> + struct jbd_revoke_record_s *record; >> + struct buffer_head *bh; >> + record = (struct jbd_revoke_record_s *)list_entry; >> + bh = __find_get_block(journal->j_fs_dev, >> + record->blocknr, >> + journal->j_blocksize); >> + if (bh) { >> + clear_buffer_revoked(bh); >> + __brelse(bh); >> + } >> + } >> + } >> +} >> + >> /* journal_switch_revoke table select j_revoke for next transaction >> * we do not want to suspend any processing until all revokes are >> * written -bzzz >> diff --git a/include/linux/jbd.h b/include/linux/jbd.h >> index 0f9f0b6..5aa964e 100644 >> --- a/include/linux/jbd.h >> +++ b/include/linux/jbd.h >> @@ -918,6 +918,7 @@ extern int journal_set_revoke(journal_t *, unsigned int, tid_t); >> extern int journal_test_revoke(journal_t *, unsigned int, tid_t); >> extern void journal_clear_revoke(journal_t *); >> extern void journal_switch_revoke_table(journal_t *journal); >> +extern void journal_clear_buffer_revoked_flags(journal_t *journal); >> >> /* >> * The log thread user interface: >> -- >> 1.7.5.1 >> > -- > Jan Kara <jack@suse.cz> > SUSE Labs, CR >
On Nov 21, 2011, at 8:55 PM, Yongqiang Yang wrote:
> YES! I had sent out the patch for jbd2. It seems that Ted is busy these days.
I generally start integrating patches for the next merge window after -rc3.
I've currently been more focused on the 1.42 e2fsprogs release.
-- Ted
--
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
diff --git a/fs/jbd/commit.c b/fs/jbd/commit.c index 8799207..f2b9a57 100644 --- a/fs/jbd/commit.c +++ b/fs/jbd/commit.c @@ -392,6 +392,12 @@ void journal_commit_transaction(journal_t *journal) jbd_debug (3, "JBD: commit phase 1\n"); /* + * Clear revoked flag to reflect there is no revoked buffers + * in the next transaction which is going to be started. + */ + journal_clear_buffer_revoked_flags(journal); + + /* * Switch to a new revoke table. */ journal_switch_revoke_table(journal); diff --git a/fs/jbd/revoke.c b/fs/jbd/revoke.c index 034eb82..501d363 100644 --- a/fs/jbd/revoke.c +++ b/fs/jbd/revoke.c @@ -47,6 +47,10 @@ * overwriting the new data. We don't even need to clear the revoke * bit here. * + * We cache revoke status of a buffer in the current transaction in b_states + * bits. As the name says, revokevalid flag indicates that the cached revoke + * status of a buffer is valid and we can rely on the cached status. + * * Revoke information on buffers is a tri-state value: * * RevokeValid clear: no cached revoke status, need to look it up @@ -474,6 +478,36 @@ int journal_cancel_revoke(handle_t *handle, struct journal_head *jh) return did_revoke; } +/* + * journal_clear_revoked_flag clears revoked flag of buffers in + * revoke table to reflect there is no revoked buffers in the next + * transaction which is going to be started. + */ +void journal_clear_buffer_revoked_flags(journal_t *journal) +{ + struct jbd_revoke_table_s *revoke = journal->j_revoke; + int i = 0; + + for (i = 0; i < revoke->hash_size; i++) { + struct list_head *hash_list; + struct list_head *list_entry; + hash_list = &revoke->hash_table[i]; + + list_for_each(list_entry, hash_list) { + struct jbd_revoke_record_s *record; + struct buffer_head *bh; + record = (struct jbd_revoke_record_s *)list_entry; + bh = __find_get_block(journal->j_fs_dev, + record->blocknr, + journal->j_blocksize); + if (bh) { + clear_buffer_revoked(bh); + __brelse(bh); + } + } + } +} + /* journal_switch_revoke table select j_revoke for next transaction * we do not want to suspend any processing until all revokes are * written -bzzz diff --git a/include/linux/jbd.h b/include/linux/jbd.h index 0f9f0b6..5aa964e 100644 --- a/include/linux/jbd.h +++ b/include/linux/jbd.h @@ -918,6 +918,7 @@ extern int journal_set_revoke(journal_t *, unsigned int, tid_t); extern int journal_test_revoke(journal_t *, unsigned int, tid_t); extern void journal_clear_revoke(journal_t *); extern void journal_switch_revoke_table(journal_t *journal); +extern void journal_clear_buffer_revoked_flags(journal_t *journal); /* * The log thread user interface:
Currently, we clear revoked flag only when a block is reused. However, this can tigger a false journal error. Consider a situation when a block is used as a meta block and is deleted(revoked) in ordered mode, then the block is allocated as a data block to a file. At this moment, user changes the file's journal mode from ordered to journaled and truncates the file. The block will be considered re-revoked by journal because it has revoked flag still pending from the last transaction and an assertion triggers. We fix the problem by keeping the revoked status more uptodate - we clear revoked flag when switching revoke tables to reflect there is no revoked buffers in current transaction any more. Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- fs/jbd/commit.c | 6 ++++++ fs/jbd/revoke.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/jbd.h | 1 + 3 files changed, 41 insertions(+), 0 deletions(-)