On Wed, Nov 28, 2018 at 7:09 PM David Sterba <dsterba@xxxxxxx> wrote: > > On Tue, Nov 27, 2018 at 03:08:08PM -0500, Josef Bacik wrote: > > On Tue, Nov 27, 2018 at 07:59:42PM +0000, Chris Mason wrote: > > > On 27 Nov 2018, at 14:54, Josef Bacik wrote: > > > > > > > On Tue, Nov 27, 2018 at 10:26:15AM +0200, Nikolay Borisov wrote: > > > >> > > > >> > > > >> On 21.11.18 г. 21:09 ч., Josef Bacik wrote: > > > >>> The cleaner thread usually takes care of delayed iputs, with the > > > >>> exception of the btrfs_end_transaction_throttle path. The cleaner > > > >>> thread only gets woken up every 30 seconds, so instead wake it up to > > > >>> do > > > >>> it's work so that we can free up that space as quickly as possible. > > > >> > > > >> Have you done any measurements how this affects the overall system. I > > > >> suspect this introduces a lot of noise since now we are going to be > > > >> doing a thread wakeup on every iput, does this give a chance to have > > > >> nice, large batches of iputs that the cost of wake up can be > > > >> amortized > > > >> across? > > > > > > > > I ran the whole patchset with our A/B testing stuff and the patchset > > > > was a 5% > > > > win overall, so I'm inclined to think it's fine. Thanks, > > > > > > It's a good point though, a delayed wakeup may be less overhead. > > > > Sure, but how do we go about that without it sometimes messing up? In practice > > the only time we're doing this is at the end of finish_ordered_io, so likely to > > not be a constant stream. I suppose since we have places where we force it to > > run that we don't really need this. IDK, I'm fine with dropping it. Thanks, > > The transaction thread wakes up cleaner periodically (commit interval, > 30s by default), so the time to process iputs is not unbounded. > > I have the same concerns as Nikolay, coupling the wakeup to all delayed > iputs could result in smaller batches. But some of the callers of > btrfs_add_delayed_iput might benefit from the extra wakeup, like > btrfs_remove_block_group, so I don't want to leave the idea yet. I'm curious, why do you think it would benefit btrfs_remove_block_group()? -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”
