Message-ID: <20191023132719.GB31271@quack2.suse.cz>
Date:   Wed, 23 Oct 2019 15:27:19 +0200
From:   Jan Kara <jack@...e.cz>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 18/22] jbd2: Reserve space for revoke descriptor blocks

On Mon 21-10-19 17:47:54, Theodore Y. Ts'o wrote:
> On Fri, Oct 04, 2019 at 12:06:04AM +0200, Jan Kara wrote:
> > Extend functions for starting, extending, and restarting transaction
> > handles to take number of revoke records handle must be able to
> > accommodate. These functions then make sure transaction has enough
> > credits to be able to store resulting revoke descriptor blocks. Also
> > revoke code tracks number of revoke records created by a handle to catch
> > situation where some place didn't reserve enough space for revoke
> > records. Similarly to standard transaction credits, space for unused
> > reserved revoke records is released when the handle is stopped.
> > 
> > On the ext4 side we currently take a simplistic approach of reserving
> > space for 1024 revoke records for any transaction. This grows amount of
> > credits reserved for each handle only by a few and is enough for any
> > normal workload so that we don't hit warnings in jbd2. We will refine
> > the logic in following commits.
> > 
> > Signed-off-by: Jan Kara <jack@...e.cz>
> 
> So let me summarize the way I think this commit is handling things.
> 
> 1) When a handle is created, the caller specifies how many revokes it
> plans to do.  If during the life of the handle, more than this number
> of revokes are done, a warning will be emited.

Correct.

> 2) For the purposes of reserving transaction credits, when we start
> the handle we assume the worst case number of number of revoke
> descriptors necessary, and we reserve that much space, and we add it
> to t_oustanding_credits.

Again correct.

> 3) When we stop the handle, we decrement t_outstanding_credits by the
> number of blocks that were originally reserved for this handle --- but
> *not* the number of worst case revoke descriptor blocks needed.  Which
> means that after the handle is started and then closed,
> t_outstanding_credits will be increased by ROUND_UP((max # of revoked
> blocks) / # of revoke blocks per block group descriptor).
> 
> If we delete a large number of files which are but a single 4k block
> in data=journal mode, each deleted file will increase
> t_outstanding_credits by one block, even though we won't be using
> anywhere *near* that number of blocks for revoke blocks.  So we will
> end up closing the transactions *much* earlier than we would have.

Right. Any handle that revokes at least one block will reserve at least one
block for revoke descriptor. I agree that will overestimate number of
necessary revoke blocks heavily in some cases. If you think that's
problematic, I can refine the logic so that rounding errors don't
accumulate that much (probably by tracking exact number of revokes in the
transaction).

> It also means that t_outstanding_credits will be a much higher number
> that we would ever need, so it's not clear to me why it's worth it to
> decrement t_outstanding_credits in jbd2_journal_get_descriptor_buffer()
> and warn if it is less than zero. 

Well, that tracking is a sanity check that we did reserve enough descriptor
blocks for each transaction.

> And it goes back to the question I had asked earler: "so what is the
> formal definition of t_outstanding_credits after this patch series,
> anyway"?

That should be answered in my previous answer.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists