On Wed, Feb 01, 2012 at 05:40:25PM -0800, Roland Dreier wrote: > and it seems that two different things are being conflated. On the one hand, > we have the following code for VPD page B0h (block limits) handling: > > /* > * Set MAXIMUM TRANSFER LENGTH > */ > put_unaligned_be32(dev->se_sub_dev->se_dev_attrib.max_sectors, &buf[8]); > > IOW we advertise max_sectors as the biggest IO an initiator should send us. > > OTOH iblock initializes max_sectors with: > > limits->max_hw_sectors = queue_max_hw_sectors(q); > limits->max_sectors = queue_max_sectors(q); > > so max_sectors is limited to what the backend device handles. I have fixed this up in the current development tree, it now reads: limits->max_hw_sectors = UINT_MAX; limits->max_sectors = UINT_MAX; > > And then in target_core_iblock.c: > > bio = bio_alloc_bioset(GFP_NOIO, sg_num, ib_dev->ibd_bio_set); > > where sg_num is task->task_sg_nents. But that bio_alloc will always > fail if sg_num > BIO_MAX_PAGES, ie 256. That looks like it's still a bug with the new code - we'd need to allocate max BIO_MAX_PAGES in a go. > So my conclusion is that we really need a distinction between the > backend max_sectors (ie what we use to decide how many tasks to > allocate) and the frontend max_sectors (ie what we advertise to the > initiator). Does it make sense split out the fields in se_dev_attrib > for the two usages? The right fix is to kill the field entirely and make sure the backends can handled arbitrary sized requests. Then as a next step kill the whole task indirection. If it wasn't for the rarely used pscsi backend this could have easily be done long time ago. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
