| Message ID | 20110427134150.GA15413@infradead.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, 2011-04-27 at 09:41 -0400, Christoph Hellwig wrote: > tc->block_shift is the shift to get from a sector to the block_size, > and it doesn't make any sense to apply that to the block size. > Without this I get overflows of the optimal I/O size queue limit > when using large block sizes in dm-thinp. NACK. >From looking at dm-raid and dm-stripe it appears that the blk_limits_io_opt function is expecting the size to be in bytes, not sectors. static void raid_io_hints(struct dm_target *ti, struct queue_limits *limits) { struct raid_set *rs = ti->private; unsigned chunk_size = rs->md.chunk_sectors << 9; raid5_conf_t *conf = rs->md.private; blk_limits_io_min(limits, chunk_size); blk_limits_io_opt(limits, chunk_size * (conf->raid_disks - conf->max_degraded)); } static void stripe_io_hints(struct dm_target *ti, struct queue_limits *limits) { struct stripe_c *sc = ti->private; unsigned chunk_size = (sc->chunk_mask + 1) << 9; blk_limits_io_min(limits, chunk_size); blk_limits_io_opt(limits, chunk_size * sc->stripes); } tc->block_size is in sectors (you are passing sectors on the target line?). What's probably happening here is we should be doing: blk_limits_io_opt(limits, min(<some theoretical max>, tc->block_size << SECTOR_SHIFT)); - Joe -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, Apr 27, 2011 at 06:25:25PM +0100, Joe Thornber wrote: > tc->block_size is in sectors (you are passing sectors on the target > line?). > > What's probably happening here is we should be doing: > > blk_limits_io_opt(limits, min(<some theoretical max>, tc->block_size << SECTOR_SHIFT)); Yes, that should do it. I don't even think we need the max, the optimum I/O size is a 32-bit value and we'll reach the limit of the possible block sizes much earlier. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, 2011-04-28 at 04:42 -0400, Christoph Hellwig wrote: > On Wed, Apr 27, 2011 at 06:25:25PM +0100, Joe Thornber wrote: > > tc->block_size is in sectors (you are passing sectors on the target > > line?). > > > > What's probably happening here is we should be doing: > > > > blk_limits_io_opt(limits, min(<some theoretical max>, tc->block_size << SECTOR_SHIFT)); > > Yes, that should do it. I don't even think we need the max, the optimum > I/O size is a 32-bit value and we'll reach the limit of the possible > block sizes much earlier. I'm tempted to just say min(16M, tc->block_size << SECTOR_SHIFT). Does this sound reasonable to you? -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Apr 28, 2011 at 09:47:33AM +0100, Joe Thornber wrote: > I'm tempted to just say min(16M, tc->block_size << SECTOR_SHIFT). Does > this sound reasonable to you? If you have a good reason for the limit go for it. But please add a comment explaining why the limit is there, so people running into it later on won't be completely puzzled. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Index: linux-2.6/drivers/md/dm-thin-prov.c =================================================================== --- linux-2.6.orig/drivers/md/dm-thin-prov.c 2011-04-27 15:30:22.798345522 +0200 +++ linux-2.6/drivers/md/dm-thin-prov.c 2011-04-27 15:30:49.084869781 +0200 @@ -641,7 +641,7 @@ thinp_io_hints(struct dm_target *ti, str struct thinp_c *tc = ti->private; blk_limits_io_min(limits, 0); - blk_limits_io_opt(limits, tc->block_size << tc->block_shift); + blk_limits_io_opt(limits, tc->block_size); } static int thinp_iterate_devices(struct dm_target *ti,
tc->block_shift is the shift to get from a sector to the block_size, and it doesn't make any sense to apply that to the block size. Without this I get overflows of the optimal I/O size queue limit when using large block sizes in dm-thinp. Signed-off-by: Christoph Hellwig <hch@lst.de> -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel