On Wed, Sep 10 2008, Hugh Dickins wrote: > On Wed, 10 Sep 2008, Jens Axboe wrote: > > On Tue, Sep 09 2008, Hugh Dickins wrote: > > > > > It seems odd to me that the data-less blkdev_issue_discard() is limited > > > at all by max_hw_sectors; but I'm guessing there's a good reason, safety > > > perhaps, which has forced you to that. > > > > The discard request needs to be turned into a hw command at some point, > > and for that we still need to fit the offset and size in there. So we > > are still limited by 32MB commands on sata w/lba48, even though we are > > not moving any data. Suboptimal, but... > > ... makes good sense, thanks. > > > > Here's the proposed patch, or combination of patches: the blkdev and > > > swap parts should certainly be separated. Advice welcome - thanks! > > > > I'll snatch up the blk bits and put them in for-2.6.28. OK if I add your > > SOB to that? > > That would be great. Thanks a lot for all your comments, I'd been > expecting a much rougher ride! If you've not already put it in, > here's that subset of the patch - change it around as you wish. > > > [PATCH] block: adjust blkdev_issue_discard for swap > > Three mods to blkdev_issue_discard(), thinking ahead to its use on swap: > > 1. Add gfp_mask argument, so swap allocation can use it where GFP_KERNEL > might deadlock but GFP_NOIO is safe. > > 2. Enlarge nr_sects argument from unsigned to sector_t: unsigned long is > enough to cover a whole swap area, but sector_t suits any partition. > > 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. > > Change sb_issue_discard()'s nr_blocks to sector_t too; but no need seen > for a gfp_mask there, just pass GFP_KERNEL down to blkdev_issue_discard(). > > Signed-off-by: Hugh Dickins <hugh@xxxxxxxxxxx> Hugh, I applied this - but on 2nd though, I killed the cond_resched() for two reasons: - We should only add stuff like that if it's known problematic - We'll be throttling on the request allocation eventually, once we get 128 of these in flight. So if this turns out to be a problem, we can revisit the cond_resched() solution. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
