[PATCH] JFFS2: Prevent CPU starvation during garbage collection.

[PATCH] JFFS2: Prevent CPU starvation during garbage collection.

[Mark Tomlinson]

The JFFS2 garbage collection checks every file on the system for correct
checksums and cleaning up anything that it needs to. It sleeps for 50ms
between each file, but within each file it remains busy.

As far as I can tell, there is no reason to not reschedule while checking a
file, by putting a cond_resched() inside jfffs2_build_inode_fragtree(). In
fact, jffs2_get_inode_nodes() will call cond_resched(), which is done
immediately before jffs2_build_inode_fragtree().

With this extra cond_resched() between reading file blocks, there is a
dramatic improvement in latency for any processes which are running during
this initial garbage collection phase.

Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>
---
 fs/jffs2/readinode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index 28e0aab..78c526a 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -536,6 +536,7 @@ static int jffs2_build_inode_fragtree(struct jffs2_sb_info *c,
 				jffs2_free_tmp_dnode_info(this);
 			}
 			this = vers_next;
+			cond_resched();
 		}
 	}
 	return 0;
-- 
2.5.0

Re: [PATCH] JFFS2: Prevent CPU starvation during garbage collection.

[Brian Norris]

+ David

I'm not maintaining JFFS2, but if it's exceedingly obvious (this patch
might qualify), I might take patches.

On Wed, Aug 05, 2015 at 04:40:30PM +1200, Mark Tomlinson wrote:
> The JFFS2 garbage collection checks every file on the system for correct
> checksums and cleaning up anything that it needs to. It sleeps for 50ms
> between each file, but within each file it remains busy.
> 
> As far as I can tell, there is no reason to not reschedule while checking a
> file, by putting a cond_resched() inside jfffs2_build_inode_fragtree(). In
> fact, jffs2_get_inode_nodes() will call cond_resched(), which is done
> immediately before jffs2_build_inode_fragtree().
> 
> With this extra cond_resched() between reading file blocks, there is a
> dramatic improvement in latency for any processes which are running during
> this initial garbage collection phase.
> 
> Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>
> ---
>  fs/jffs2/readinode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
> index 28e0aab..78c526a 100644
> --- a/fs/jffs2/readinode.c
> +++ b/fs/jffs2/readinode.c
> @@ -536,6 +536,7 @@ static int jffs2_build_inode_fragtree(struct jffs2_sb_info *c,
>  				jffs2_free_tmp_dnode_info(this);
>  			}
>  			this = vers_next;
> +			cond_resched();
>  		}
>  	}
>  	return 0;

Obligatory question: did you test this?

Brian

Re: [PATCH] JFFS2: Prevent CPU starvation during garbage collection.

[Mark Tomlinson]

I have tested that this does reduce latency for other processes. In our 
embedded system, one process tries to wake up every 1000ms using poll(). 
During JFFS2's period of scanning, it could take 2500ms before that 
process got the CPU. After this patch, the longest I ever saw was 
1050ms, which is much more acceptable to us.

The only question that remains is whether allowing a reschedule at this 
point is OK to do or not. My reasoning below says that it is, but I 
could be mistaken. I have not seen any bad behaviour while running with 
this patch.

On 08/08/2015 05:21 AM, Brian Norris wrote:
> + David
>
> I'm not maintaining JFFS2, but if it's exceedingly obvious (this patch
> might qualify), I might take patches.
>
> On Wed, Aug 05, 2015 at 04:40:30PM +1200, Mark Tomlinson wrote:
>> The JFFS2 garbage collection checks every file on the system for correct
>> checksums and cleaning up anything that it needs to. It sleeps for 50ms
>> between each file, but within each file it remains busy.
>>
>> As far as I can tell, there is no reason to not reschedule while checking a
>> file, by putting a cond_resched() inside jfffs2_build_inode_fragtree(). In
>> fact, jffs2_get_inode_nodes() will call cond_resched(), which is done
>> immediately before jffs2_build_inode_fragtree().
>>
>> With this extra cond_resched() between reading file blocks, there is a
>> dramatic improvement in latency for any processes which are running during
>> this initial garbage collection phase.
>>
>> Signed-off-by: Mark Tomlinson <mark.tomlinson@alliedtelesis.co.nz>
>> ---
>>   fs/jffs2/readinode.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
>> index 28e0aab..78c526a 100644
>> --- a/fs/jffs2/readinode.c
>> +++ b/fs/jffs2/readinode.c
>> @@ -536,6 +536,7 @@ static int jffs2_build_inode_fragtree(struct jffs2_sb_info *c,
>>   				jffs2_free_tmp_dnode_info(this);
>>   			}
>>   			this = vers_next;
>> +			cond_resched();
>>   		}
>>   	}
>>   	return 0;
> Obligatory question: did you test this?
>
> Brian