On Fri, Apr 12, 2019 at 01:17:40PM +0300, Nikolay Borisov wrote: > > > On 10.04.19 г. 22:56 ч., Josef Bacik wrote: > > When diagnosing a slowdown of generic/224 I noticed we were wasting a > > lot of time in shrink_delalloc() despite all writes being O_DIRECT > > writes. O_DIRECT writes still have outstanding extents, but obviously > > cannot be directly flushed, instead we need to wait on their > > corresponding ordered extent. Track the outstanding odirect write bytes > > and if this amount is higher than the delalloc bytes in the system go > > ahead and force us to wait on the ordered extents. > > This is way too sparse. I've been running generic/224 to try and > reproduce your slowdown. So far I can confirm that this test exhibits > drastic swings in performance - I've seen it complete from 30s up to > 300s. I've also been taking an offcputime[0] measurements in the case > where high completion times were observed but so far I haven't really > seen shrink_delalloc standing out. > > Provide more information how you measured the said slowdown as well as > more information in the changelog about why it's happening. At the very > least this could be split into 2 patches: > > 1. Could add the percpu counter init + modification in ordered extent > routines > > 2. Should add the logic in shrink_delalloc. Ideally that patch will > include detailed explanation of how the problem manifests.
I don't think splitting the init code is required here, I did not find it distracting while reading the patch. The 'ideally' part of your comment is be something that we should not ask for. Missing or vague explanation for change like that is a bad practice. Josef, please update the changelog and resend, thanks.
