ocfs2: Support nested transactions
PATCH 1/1 ocfs2: Support nested transactions
OCFS2 can easily support nested transactions. We just have to take care and not spoil statistics acquire semaphore unnecessarily.
--- fs/ocfs2/journal.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)
fs/ocfs2/journal.c +7 -7
@@ -256,11 +256,9 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)BUG_ON (osb->journal->j_state ==OCFS2_JOURNAL_FREE );BUG_ON (max_buffs <=0 ); -/* JBD might support this, but our journalling code doesn't yet. */ -if (journal_current_handle ()) { -mlog (ML_ERROR ,"Recursive transaction attempted!\n" ); -BUG (); - } +/* Nested transaction? Just return the handle... */ +if (journal_current_handle ()) +return journal_start (journal, max_buffs);down_read (&osb->journal->j_trans_barrier);
@@ -285,16 +283,18 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)int ocfs2_commit_trans (struct ocfs2_super *osb,handle_t *handle) { -int ret; +int ret, nested;struct ocfs2_journal *journal = osb->journal;BUG_ON (!handle); + nested = handle->h_ref >1 ; ret =journal_stop (handle);if (ret <0 )mlog_errno (ret); -up_read (&journal->j_trans_barrier); +if (!nested) +up_read (&journal->j_trans_barrier);return ret; }
Signature · 1 line
1.5.2.4
PATCH 1/1 ocfs2: Support nested transactions
Quoted 2 lines · Jan Kara
OCFS2 can easily support nested transactions. We just have to take care and not spoil statistics acquire semaphore unnecessarily.
I'm guessing this is required for the quota code, because it
wants to start its own transaction underneath the transaction ocfs2 is doing? Is there a way to have ocfs2 just pass the handle to the quota op (I'm guessing not because it's in the VFS)?
This just scares me a little.
Quoted 35 lines
Signed-off-by Jan Kara <[email protected]>--- fs/ocfs2/journal.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)fs/ocfs2/journal.c +5
-6@@ -256,11 +256,9 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)BUG_ON (osb->journal->j_state ==OCFS2_JOURNAL_FREE );BUG_ON (max_buffs <=0 ); -/* JBD might support this, but our journalling code doesn't yet. */ -if (journal_current_handle ()) { -mlog (ML_ERROR ,"Recursive transaction attempted!\n" ); -BUG (); - } +/* Nested transaction? Just return the handle... */ +if (journal_current_handle ()) +return journal_start (journal, max_buffs);down_read (&osb->journal->j_trans_barrier);@@ -285,16 +283,18 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)int ocfs2_commit_trans (struct ocfs2_super *osb,handle_t *handle) { -int ret; +int ret, nested;struct ocfs2_journal *journal = osb->journal;BUG_ON (!handle); + nested = handle->h_ref >1 ;
fs/ocfs2/journal.c handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs) · line 300
Quoted 10 lines
fs/ocfs2/journal.c +2
-1ret =journal_stop (handle);if (ret <0 )mlog_errno (ret); -up_read (&journal->j_trans_barrier); +if (!nested) +up_read (&journal->j_trans_barrier);return ret; }
If we're doing to do this, I'd like to see a check inocfs2_commit_trans() to make sure we're running ABBA nested and not ABAB overlapping. Perhaps a flag on osb->journal somehow?
Joel
Signature · 8 lines
"Vote early and vote often."
- Al Capone
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127PATCH 1/1 ocfs2: Support nested transactions
Quoted 8 lines · Joel Becker
Quoted 2 lines · Jan Kara
OCFS2 can easily support nested transactions. We just have to take care and not spoil statistics acquire semaphore unnecessarily.
I'm guessing this is required for the quota code, because it
wants to start its own transaction underneath the transaction ocfs2 is doing? Is there a way to have ocfs2 just pass the handle to the quota op (I'm guessing not because it's in the VFS)?
Exactly and moreover there are paths in VFS which can get to that place
without starting a transaction and other paths which get there with a transaction started and it's easier to not have to care.
Quoted 1 line
This just scares me a little.
Well, unless OCFS2 does with transactions something non-standard (which I
don't think so), it should not be a problem :) Quota on ext3/4 does exactly the same thing.
Quoted 43 lines
Quoted 43 lines
Signed-off-by Jan Kara <[email protected]>--- fs/ocfs2/journal.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)fs/ocfs2/journal.c +7
-7@@ -256,11 +256,9 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)BUG_ON (osb->journal->j_state ==OCFS2_JOURNAL_FREE );BUG_ON (max_buffs <=0 ); -/* JBD might support this, but our journalling code doesn't yet. */ -if (journal_current_handle ()) { -mlog (ML_ERROR ,"Recursive transaction attempted!\n" ); -BUG (); - } +/* Nested transaction? Just return the handle... */ +if (journal_current_handle ()) +return journal_start (journal, max_buffs);down_read (&osb->journal->j_trans_barrier);@@ -285,16 +283,18 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs)int ocfs2_commit_trans (struct ocfs2_super *osb,handle_t *handle) { -int ret; +int ret, nested;struct ocfs2_journal *journal = osb->journal;BUG_ON (!handle); + nested = handle->h_ref >1 ; ret =journal_stop (handle);if (ret <0 )mlog_errno (ret); -up_read (&journal->j_trans_barrier); +if (!nested) +up_read (&journal->j_trans_barrier);
Quoted 6 lines
fs/ocfs2/journal.c handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs) · line 300
Quoted 2 lines
fs/ocfs2/journal.c +0
-0return ret; }If we're doing to do this, I'd like to see a check in
ocfs2_commit_trans()to make sure we're running ABBA nested and not ABAB overlapping. Perhaps a flag onosb->journalsomehow?
Hmm, how do you find out to which ocfs2_start_trans() doesocfs2_commit_trans() belong? Note that for nested transaction, number of credits requested is actually discarded so the only thing it really does is just increase the reference count of the handle.
As I'm looking into my changes, it could even be cleaned up a bit so thatj_trans_barrier is taken and dropped always and only increasing of j_num_trans would be conditioned on reference being the last one... I can do that if you like it more like that.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR