diff mbox

JFFS2: Prevent CPU starvation during garbage collection.

Message ID 1438749630-1656-1-git-send-email-mark.tomlinson@alliedtelesis.co.nz
State Not Applicable
Delegated to: David Woodhouse
Headers show

Commit Message

Mark Tomlinson Aug. 5, 2015, 4:40 a.m. UTC
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(+)

Comments

Brian Norris Aug. 7, 2015, 5:21 p.m. UTC | #1
+ 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
Mark Tomlinson Aug. 9, 2015, 10:24 p.m. UTC | #2
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
diff mbox

Patch

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;