On 05/17/2016 01:16 PM, Bart Van Assche wrote: > On 05/16/2016 12:02 PM, Martin K. Petersen wrote: >>>>>>> "Bart" == Bart Van Assche <bart.vanassche@xxxxxxxxxxx> writes: >> >> Bart> That's a good catch. But seeing this patch makes me wonder whether >> Bart> this patch introduces a 64-bit division? If so, I'm afraid this >> Bart> patch will make 32-bit users unhappy. Have you considered to use >> Bart> do_div() or >> (ilog2(block_size) - 9) instead? For the latter >> Bart> alternative no 64-bit cast is needed. >> >> Please use logical_to_sectors(). > > Hello Martin, > > For SCSI initiator devices the logical block size is stored in struct > scsi_device. logical_to_sectors() gets the logical block size from > struct scsi_device. LIO however stores the logical block size in struct > se_dev_attrib. If we want to keep SCSI initiator and SCSI target headers > separate I think we will need two logical_to_sectors() functions - one > for SCSI initiator devices and one for SCSI target devices. > Or block layer helpers? Something like the attached patch. It is a compile only tested patch (just for show and not for submission as I would break it up and test, etc) that would move the scsi converter to the block layer, and convert the target layer and scsi to use it.
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 654630b..d405400 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -148,7 +148,7 @@ static inline int scsi_medium_access_command(struct scsi_cmnd *scmd) static inline sector_t logical_to_sectors(struct scsi_device *sdev, sector_t blocks) { - return blocks << (ilog2(sdev->sector_size) - 9); + return blk_logical_to_sector(blocks, sdev->sector_size); } /* diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index a4046ca..95d6112 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -821,13 +821,15 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) * in ATA and we need to set TPE=1 */ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, - struct request_queue *q, int block_size) + struct request_queue *q) { + unsigned short block_size = queue_logical_block_size(q); + if (!blk_queue_discard(q)) return false; - attrib->max_unmap_lba_count = (q->limits.max_discard_sectors << 9) / - block_size; + attrib->max_unmap_lba_count = queue_sector_to_logical(q, + q->limits.max_discard_sectors); /* * Currently hardcoded to 1 in Linux/SCSI code.. */ @@ -846,16 +848,7 @@ EXPORT_SYMBOL(target_configure_unmap_from_queue); */ sector_t target_to_linux_sector(struct se_device *dev, sector_t lb) { - switch (dev->dev_attrib.block_size) { - case 4096: - return lb << 3; - case 2048: - return lb << 2; - case 1024: - return lb << 1; - default: - return lb; - } + return blk_logical_to_sector(lb, dev->dev_attrib.block_size); } EXPORT_SYMBOL(target_to_linux_sector); diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 75f0f08..7929186 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -161,8 +161,7 @@ static int fd_configure_device(struct se_device *dev) dev_size, div_u64(dev_size, fd_dev->fd_block_size), fd_dev->fd_block_size); - if (target_configure_unmap_from_queue(&dev->dev_attrib, q, - fd_dev->fd_block_size)) + if (target_configure_unmap_from_queue(&dev->dev_attrib, q)) pr_debug("IFILE: BLOCK Discard support available," " disabled by default\n"); /* diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 026a758..3cbe060 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -121,8 +121,7 @@ static int iblock_configure_device(struct se_device *dev) dev->dev_attrib.hw_max_sectors = queue_max_hw_sectors(q); dev->dev_attrib.hw_queue_depth = q->nr_requests; - if (target_configure_unmap_from_queue(&dev->dev_attrib, q, - dev->dev_attrib.hw_block_size)) + if (target_configure_unmap_from_queue(&dev->dev_attrib, q)) pr_debug("IBLOCK: BLOCK Discard support available," " disabled by default\n"); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 669e419..55cb34b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1210,6 +1210,30 @@ static inline unsigned short bdev_logical_block_size(struct block_device *bdev) return queue_logical_block_size(bdev_get_queue(bdev)); } +static inline sector_t blk_logical_to_sector(sector_t blocks, + unsigned short block_size) +{ + return blocks << (ilog2(block_size) - 9); +} + +static inline sector_t queue_logical_to_sector(struct request_queue *q, + sector_t blocks) +{ + return blk_logical_to_sector(blocks, queue_logical_block_size(q)); +} + +static inline sector_t blk_sector_to_logical(sector_t sectors, + unsigned short block_size) +{ + return sectors >> (ilog2(block_size) - 9); +} + +static inline sector_t queue_sector_to_logical(struct request_queue *q, + sector_t sectors) +{ + return blk_sector_to_logical(sectors, queue_logical_block_size(q)); +} + static inline unsigned int queue_physical_block_size(struct request_queue *q) { return q->limits.physical_block_size; diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h index 28ee5c2..711322a 100644 --- a/include/target/target_core_backend.h +++ b/include/target/target_core_backend.h @@ -96,6 +96,6 @@ sense_reason_t passthrough_parse_cdb(struct se_cmd *cmd, bool target_sense_desc_format(struct se_device *dev); sector_t target_to_linux_sector(struct se_device *dev, sector_t lb); bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib, - struct request_queue *q, int block_size); + struct request_queue *q); #endif /* TARGET_CORE_BACKEND_H */