Strange automatic GC threshold ?

Strange automatic GC threshold ?

[Joakim Tjernlund]

In jffs2_thread_should_wake() there is this test to
wake up GC thread:
	/* dirty_size contains blocks on erase_pending_list
	 * those blocks are counted in c->nr_erasing_blocks.
	 * If one block is actually erased, it is not longer counted as dirty_space
	 * but it is counted in c->nr_erasing_blocks, so we add it and subtract it
	 * with c->nr_erasing_blocks * c->sector_size again.
	 * Blocks on erasable_list are counted as dirty_size, but not in c->nr_erasing_blocks
	 * This helps us to force gc and pick eventually a clean block to spread the load.
	 */
	dirty = c->dirty_size + c->erasing_size - c->nr_erasing_blocks * c->sector_size;

	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
			(dirty > c->nospc_dirty_size))

The && in the above if means that no matter how dirty the FS is, it wont
trigger GC until there is a very low count of free blocks as well(24 in my
case out of 1003). I wonder if that && should be a || instead?

 Jocke

Re: Strange automatic GC threshold ?

[MikeW]

Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:

> 
> In jffs2_thread_should_wake() there is this test to
> wake up GC thread:
> 
> 	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
> 			(dirty > c->nospc_dirty_size))
> 
> The && in the above if means that no matter how dirty the FS is, it wont
> trigger GC until there is a very low count of free blocks as well(24 in my
> case out of 1003). I wonder if that && should be a || instead?
> 
>  Jocke

In the interests of clarity and robustness to edits, there should
be (  ) around the comparison subexpression at any rate.

Regards,
MikeW

Re: Strange automatic GC threshold ?

[Josh Boyer]

On Mon, 2007-03-19 at 16:49 +0000, MikeW wrote:
> Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:
> 
> > 
> > In jffs2_thread_should_wake() there is this test to
> > wake up GC thread:
> > 
> > 	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
> > 			(dirty > c->nospc_dirty_size))
> > 
> > The && in the above if means that no matter how dirty the FS is, it wont
> > trigger GC until there is a very low count of free blocks as well(24 in my
> > case out of 1003). I wonder if that && should be a || instead?
> > 
> >  Jocke
> 
> In the interests of clarity and robustness to edits, there should
> be (  ) around the comparison subexpression at any rate.

Patches welcome :)

josh

Re: Strange automatic GC threshold ?

[Joakim Tjernlund]

On Mon, 2007-03-19 at 12:03 -0500, Josh Boyer wrote:
> On Mon, 2007-03-19 at 16:49 +0000, MikeW wrote:
> > Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:
> > 
> > > 
> > > In jffs2_thread_should_wake() there is this test to
> > > wake up GC thread:
> > > 
> > > 	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
> > > 			(dirty > c->nospc_dirty_size))
> > > 
> > > The && in the above if means that no matter how dirty the FS is, it wont
> > > trigger GC until there is a very low count of free blocks as well(24 in my
> > > case out of 1003). I wonder if that && should be a || instead?
> > > 
> > >  Jocke
> > 
> > In the interests of clarity and robustness to edits, there should
> > be (  ) around the comparison subexpression at any rate.
> 
> Patches welcome :)
> 
> josh

How about thinking about the orginal question that made me send this
mail in the first place before sending any patches? :)

 Jocke

Re: Strange automatic GC threshold ?

[Josh Boyer]

On Mon, 2007-03-19 at 18:21 +0100, Joakim Tjernlund wrote:
> On Mon, 2007-03-19 at 12:03 -0500, Josh Boyer wrote:
> > On Mon, 2007-03-19 at 16:49 +0000, MikeW wrote:
> > > Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:
> > > 
> > > > 
> > > > In jffs2_thread_should_wake() there is this test to
> > > > wake up GC thread:
> > > > 
> > > > 	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
> > > > 			(dirty > c->nospc_dirty_size))
> > > > 
> > > > The && in the above if means that no matter how dirty the FS is, it wont
> > > > trigger GC until there is a very low count of free blocks as well(24 in my
> > > > case out of 1003). I wonder if that && should be a || instead?
> > > > 
> 
> How about thinking about the orginal question that made me send this
> mail in the first place before sending any patches? :)

I have been.  That response just takes more time. :)  (And the patches
comment was about adding more (), not to you specifically.)

I'd be interested to see how the behavior of GC differs with the
proposed change.  Theoretically, it seems to make sense to allow GC to
make progress and it could help the "oh crap we're out of space" grind
that seems to occur.  My biggest concern would be if it actually made GC
harder to do in the long run as the obsolete nodes could potentially get
more scattered among the eraseblocks.

Do you (or anyone else) have a setup where you could test the change
with varying cases of write frequency and usage?

josh

Re: Strange automatic GC threshold ?

[Joakim Tjernlund]

On Mon, 2007-03-19 at 12:35 -0500, Josh Boyer wrote:
> On Mon, 2007-03-19 at 18:21 +0100, Joakim Tjernlund wrote:
> > On Mon, 2007-03-19 at 12:03 -0500, Josh Boyer wrote:
> > > On Mon, 2007-03-19 at 16:49 +0000, MikeW wrote:
> > > > Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:
> > > > 
> > > > > 
> > > > > In jffs2_thread_should_wake() there is this test to
> > > > > wake up GC thread:
> > > > > 
> > > > > 	if (c->nr_free_blocks + c->nr_erasing_blocks < c->resv_blocks_gctrigger &&
> > > > > 			(dirty > c->nospc_dirty_size))
> > > > > 
> > > > > The && in the above if means that no matter how dirty the FS is, it wont
> > > > > trigger GC until there is a very low count of free blocks as well(24 in my
> > > > > case out of 1003). I wonder if that && should be a || instead?
> > > > > 
> > 
> > How about thinking about the orginal question that made me send this
> > mail in the first place before sending any patches? :)
> 
> I have been.  That response just takes more time. :)  (And the patches
> comment was about adding more (), not to you specifically.)

I know, hence the :)

> 
> I'd be interested to see how the behavior of GC differs with the
> proposed change.  Theoretically, it seems to make sense to allow GC to
> make progress and it could help the "oh crap we're out of space" grind
> that seems to occur.  My biggest concern would be if it actually made GC
> harder to do in the long run as the obsolete nodes could potentially get
> more scattered among the eraseblocks.

Can't be worse than current behavior as now the system becomes painfully
slow long before it starts GC. Perhaps one need one threshold to start
GC due to dirtiness and another one when to stop?

> 
> Do you (or anyone else) have a setup where you could test the change
> with varying cases of write frequency and usage?

Sorry, but no.

> 
> josh
> 
> 
> 

Re: Strange automatic GC threshold ?

[Josh Boyer]

On Mon, 2007-03-19 at 19:17 +0100, Joakim Tjernlund wrote:
> > I'd be interested to see how the behavior of GC differs with the
> > proposed change.  Theoretically, it seems to make sense to allow GC to
> > make progress and it could help the "oh crap we're out of space" grind
> > that seems to occur.  My biggest concern would be if it actually made GC
> > harder to do in the long run as the obsolete nodes could potentially get
> > more scattered among the eraseblocks.
> 
> Can't be worse than current behavior as now the system becomes painfully
> slow long before it starts GC. Perhaps one need one threshold to start
> GC due to dirtiness and another one when to stop?

Well, it can though.  If the obsolete nodes are scattered throughout the
blocks and moving them doesn't actually make any single eraseblock
totally obsolete then you've eliminated the effectiveness of GC
completely.  That problem exists today of course, but I'm concerned it
could happen with more frequency if we're GCing sooner.  Maybe I'm just
paranoid.

The often suggested idea of GCing into a completely separate block than
the one you're writing into normally might help here.  But I'm not sure
how feasible that is at this moment.

josh

Re: Strange automatic GC threshold ?

[MikeW]

Josh Boyer <jwboyer <at> linux.vnet.ibm.com> writes:

> 
> On Mon, 2007-03-19 at 19:17 +0100, Joakim Tjernlund wrote:
> > > I'd be interested to see how the behavior of GC differs with the
> > > proposed change.  Theoretically, it seems to make sense to allow GC to
> > > make progress and it could help the "oh crap we're out of space" grind
> > > that seems to occur.  My biggest concern would be if it actually made GC
> > > harder to do in the long run as the obsolete nodes could potentially get
> > > more scattered among the eraseblocks.
> > 
> > Can't be worse than current behavior as now the system becomes painfully
> > slow long before it starts GC. Perhaps one need one threshold to start
> > GC due to dirtiness and another one when to stop?
> 
> Well, it can though.  If the obsolete nodes are scattered throughout the
> blocks and moving them doesn't actually make any single eraseblock
> totally obsolete then you've eliminated the effectiveness of GC
> completely.  That problem exists today of course, but I'm concerned it
> could happen with more frequency if we're GCing sooner.  Maybe I'm just
> paranoid.
> 
> The often suggested idea of GCing into a completely separate block than
> the one you're writing into normally might help here.  But I'm not sure
> how feasible that is at this moment.
> 
> josh

Unfortunately, subjects like GC are notoriously easy to "improve" - only
to discover that the "improvement" either doesn't help, or makes things worse
or very bad in some situations.

For this, some kind of algorithm test by simulation would be a good idea.

In this way, many different scenarios can be "left to run" over many different
cases to compare performances and any delinquent behaviour.

Regards,
MikeW

Re: Strange automatic GC threshold ?

[Joakim Tjernlund]

On Mon, 2007-03-19 at 13:53 -0500, Josh Boyer wrote:
> On Mon, 2007-03-19 at 19:17 +0100, Joakim Tjernlund wrote:
> > > I'd be interested to see how the behavior of GC differs with the
> > > proposed change.  Theoretically, it seems to make sense to allow GC to
> > > make progress and it could help the "oh crap we're out of space" grind
> > > that seems to occur.  My biggest concern would be if it actually made GC
> > > harder to do in the long run as the obsolete nodes could potentially get
> > > more scattered among the eraseblocks.
> > 
> > Can't be worse than current behavior as now the system becomes painfully
> > slow long before it starts GC. Perhaps one need one threshold to start
> > GC due to dirtiness and another one when to stop?
> 

Sorry for the delay, got sucked into a black hole :)

> Well, it can though.  If the obsolete nodes are scattered throughout the
> blocks and moving them doesn't actually make any single eraseblock
> totally obsolete then you've eliminated the effectiveness of GC
> completely.  That problem exists today of course, but I'm concerned it
> could happen with more frequency if we're GCing sooner.  Maybe I'm just
> paranoid.

I hope you are just paranoid :)

I don't mind if the effectivness of GC w.r.t free space is 
somewhat reduced. Some systems(like ours) will die due to slow
FS since some apps thinks something is serioulsy wrong when
they can't communicate with another app since it is stuck in I/O.

If the current GC behaviour is important to many systems I think
we need a config option where one can enable a more agressive
GC mode that tries to keep the FS clean to maintain FS speed.

 Jocke

> 
> The often suggested idea of GCing into a completely separate block than
> the one you're writing into normally might help here.  But I'm not sure
> how feasible that is at this moment.
> 
> josh
> 
> 
> 

Re: Strange automatic GC threshold ?

[MikeW]

Joakim Tjernlund <joakim.tjernlund <at> transmode.se> writes:

> 
> On Mon, 2007-03-19 at 12:03 -0500, Josh Boyer wrote:
> 
> How about thinking about the orginal question that made me send this
> mail in the first place before sending any patches? :)
> 
>  Jocke
> 

Hey sorry - I thought there was meant to be a benefit of the "many eyes"
open source code checking, and was adding in some code resilience,
since you had drawn my attention to those few lines !!

Regards,
MikeW