On 04/04/2016 03:00 AM, Hannes Reinecke wrote:
@@ -728,6 +729,10 @@ static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
int ret = 0;
char *buf;
struct page *page = NULL;
+#ifdef CONFIG_SCSI_ZBC
+ struct blk_zone *zone;
+ unsigned long flags;
+#endif
There is a strong preference in the Linux kernel for avoiding #ifdefs
and to move code that depends on the value of a CONFIG_* variable into
a file for which the compilation depends on that CONFIG_* variable.
Please consider to move the ZBC code from sd_setup_discard_cmnd() into a
new function in sd_zbc.c.
+#ifdef CONFIG_SCSI_ZBC
+ zone = blk_lookup_zone(rq->q, sector);
+ if (!zone) {
+ ret = BLKPREP_KILL;
+ goto out;
+ }
+ spin_lock_irqsave(&zone->lock, flags);
+ if (zone->state == BLK_ZONE_BUSY) {
+ sd_printk(KERN_INFO, sdkp,
+ "Discarding busy zone %zu/%zu\n",
+ zone->start, zone->len);
+ spin_unlock_irqrestore(&zone->lock, flags);
+ ret = BLKPREP_DEFER;
+ goto out;
+ }
+ if (!blk_zone_is_smr(zone)) {
+ sd_printk(KERN_INFO, sdkp,
+ "Discarding %s zone %zu/%zu\n",
+ blk_zone_is_cmr(zone) ? "CMR" : "unknown",
+ zone->start, zone->len);
+ spin_unlock_irqrestore(&zone->lock, flags);
+ ret = BLKPREP_DONE;
+ goto out;
+ }
+ if (blk_zone_is_empty(zone)) {
+ spin_unlock_irqrestore(&zone->lock, flags);
+ ret = BLKPREP_DONE;
+ goto out;
+ }
+ if (zone->start != sector ||
+ zone->len < nr_sectors) {
+ sd_printk(KERN_INFO, sdkp,
+ "Misaligned RESET WP, start %zu/%zu "
+ "len %zu/%u\n",
+ zone->start, sector, zone->len, nr_sectors);
+ spin_unlock_irqrestore(&zone->lock, flags);
+ ret = BLKPREP_KILL;
+ goto out;
+ }
+ /*
+ * Opportunistic setting, needs to be fixed up
+ * if RESET WRITE POINTER fails.
+ */
+ zone->wp = zone->start;
+ spin_unlock_irqrestore(&zone->lock, flags);
+#endif
> cmd->cmd_len = 16;
> cmd->cmnd[0] = ZBC_OUT;
> cmd->cmnd[1] = ZO_RESET_WRITE_POINTER;
Which mechanism prevents that zone->state is modified after it has been
checked and before the RESET WRITE POINTER command has finished?
@@ -990,6 +1041,13 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCSI_LOG_HLQUEUE(2, scmd_printk(KERN_INFO, SCpnt, "block=%llu\n",
(unsigned long long)block));
+ if (sdkp->zoned == 1 || sdp->type == TYPE_ZBC) {
+ /* sd_zbc_lookup_zone lba is in block layer sector units */
+ ret = sd_zbc_lookup_zone(sdkp, rq, block, this_count);
+ if (ret != BLKPREP_OK)
+ goto out;
+ }
+
Which mechanism guarantees that the above code won't run concurrently
with zbc_parse_zones()?
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5debd49..35c75fa 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -65,6 +65,12 @@ struct scsi_disk {
struct scsi_device *device;
struct device dev;
struct gendisk *disk;
+#ifdef CONFIG_SCSI_ZBC
+ struct workqueue_struct *zone_work_q;
+ unsigned long zone_flags;
+#define SD_ZBC_ZONE_RESET 1
+#define SD_ZBC_ZONE_INIT 2
+#endif
The above two constants are only used in source file sd_zbc.c. Have you
considered to move the definition of these constants into sd_zbc.c?
+#undef SD_ZBC_DEBUG
Please use the dynamic_debug facility instead of #ifdef SD_ZBC_DEBUG +
sd_printk().
+void sd_zbc_refresh_zone_work(struct work_struct *work)
+{
+ struct zbc_update_work *zbc_work =
+ container_of(work, struct zbc_update_work, zone_work);
+ struct scsi_disk *sdkp = zbc_work->sdkp;
+ struct request_queue *q = sdkp->disk->queue;
+ unsigned int zone_buflen;
+ int ret;
+ sector_t last_sector;
+ sector_t capacity = logical_to_sectors(sdkp->device, sdkp->capacity);
+ sector_t zone_lba = sectors_to_logical(sdkp->device,
+ zbc_work->zone_sector);
+
+ zone_buflen = zbc_work->zone_buflen;
+ ret = sd_zbc_report_zones(sdkp, zone_lba, zbc_work->zone_buf,
+ zone_buflen);
+ if (ret)
+ goto done_free;
+
+ last_sector = zbc_parse_zones(sdkp, zbc_work->zone_buf, zone_buflen);
+ if (last_sector != -1 && last_sector < capacity) {
+ if (test_bit(SD_ZBC_ZONE_RESET, &sdkp->zone_flags)) {
+#ifdef SD_ZBC_DEBUG
+ sd_printk(KERN_INFO, sdkp,
+ "zones in reset, cancelling refresh\n");
+#endif
+ ret = -EAGAIN;
+ goto done_free;
+ }
+
+ zbc_work->zone_sector = last_sector;
+ queue_work(sdkp->zone_work_q, &zbc_work->zone_work);
+ /* Kick request queue to be on the safe side */
+ goto done_start_queue;
+ }
+done_free:
+ kfree(zbc_work);
+ if (test_and_clear_bit(SD_ZBC_ZONE_INIT, &sdkp->zone_flags) && ret) {
+ sd_printk(KERN_INFO, sdkp,
+ "Cancelling zone initialisation\n");
+ }
+done_start_queue:
+ if (q->mq_ops)
+ blk_mq_start_hw_queues(q);
+ else {
+ unsigned long flags;
+
+ spin_lock_irqsave(q->queue_lock, flags);
+ blk_start_queue(q);
+ spin_unlock_irqrestore(q->queue_lock, flags);
+ }
+}
Which mechanism prevents concurrent execution of
sd_zbc_refresh_zone_work() and READ and WRITE commands?
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
