Re: [RFC PATCH 4/4] xfs: add background scanning to clear EOFBLOCKS inodes

Brian Foster
2012-09-04T14:09:13Z

On 09/03/2012 01:28 AM, Dave Chinner wrote:
> On Mon, Aug 27, 2012 at 03:51:51PM -0400, Brian Foster wrote:
>> Create a delayed_work to enable background scanning and freeing
>> of EOFBLOCKS inodes. The scanner kicks in once speculative
>> preallocation occurs and stops requeueing itself when no EOFBLOCKS
>> inodes exist.
>>
>> Scans are queued on the existing syncd workqueue and the interval
>> is tied to the syncd interval, which is a default of 30s. The
>> minimum file size for a background scan is hardcoded to 100MB.
>
> I'd set it to be much longer than the xfs_syncd_centisecs period.
> The data may not be written back for a long while, and a file that
> is open, written and closed repeatedly (NFS write patterns for large
> and/or slowly written files) should not have it's preallocation
> truncated every 30s.
>
> I'd suggest that a 5 minute sweep is probably sufficiently frequent
> to avoid most fragmentation issues and long term buildup of speculative
> preallocation whilst not perturbing large/slow writers too much....
>

Any thoughts on having tunables for both values (time and min size?) on
the background scanning?

> Apart from that, there isn't anything wrong with the code, except
> I'll note again that it conflicts with my syncd removal patchset....
>

Yeah, I skimmed through those patches and noticed it would shuffle this
code around. Thanks for pointing it out. I'll keep an eye on that set.

Thanks for the review. I'll follow up with a patchset that incorporates
the feedback.

Brian

> Cheers,
>
> Dave.