Re: [RFC PATCH] discarding swap

Hugh Dickins
2008-09-11T10:48:05Z

On Thu, 11 Sep 2008, Jens Axboe wrote:
> On Wed, Sep 10 2008, Hugh Dickins wrote:
> >
> > 3. Add an occasional cond_resched() into the loop, to avoid risking bad
> > latencies when discarding a large area in small max_hw_sectors steps.
>
> Hugh, I applied this - but on 2nd though, I killed the cond_resched()
> for two reasons:

Thanks. Yes, that was definitely the most dubious part of the patch.

>
> - We should only add stuff like that if it's known problematic

Fair enough. I tend to be more proactive than that with mm loops,
and perhaps had it overmuch on my mind because the swap allocation
loop itself used to be such a prime offender.

(There's also the argument that those most worried about such latencies
will be setting CONFIG_PREEMPT=y, in which case no cond_resched() needed:
I like to give that argument some respect, but not take it too far.)

> - We'll be throttling on the request allocation eventually, once we get
> 128 of these in flight.

Yes, my worry was that if the device completes these requests quickly
enough (as we hope it, or many of them, will), blkdev_issue_discard()
may never reach that throttling, despite doing lots more than 128.

>
> So if this turns out to be a problem, we can revisit the cond_resched()
> solution.

Indeed - and it doesn't affect the blkdev_issue_discard() interface,
just its implementation.

(I'm still mulling over, in between unrelated work,
David's point on the barriers: will reply to that later.)

Hugh