[Devel] [PATCH RHEL7 COMMIT] ms/target: fix DPO and FUA bit checks

Konstantin Khorenko khorenko at virtuozzo.com
Mon Apr 2 17:28:26 MSK 2018


The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.46.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-693.21.1.vz7.46.2
------>
commit deea49e73219891239924b50190ebcbb486c220f
Author: Christoph Hellwig <hch at lst.de>
Date:   Mon Apr 2 17:28:26 2018 +0300

    ms/target: fix DPO and FUA bit checks
    
    ML: 814e5b45182f4aaf6c0b0deac7104bc2cba5109e
    
    Drivers may override the WCE flag, in which case the DPOFUA flag in
    MODE SENSE might differ from the check used to reject invalid FUA
    bits in sbc_check_dpofua.  Also now that we reject invalid FUA
    bits early there is no need to duplicate the same buggy check
    down in the fileio code.
    
    As the DPOFUA flag controls th support for FUA bits on read and
    write commands as well as DPO key off all the checks off a single
    helper, and deprecate the emulate_dpo and emulate_fua_read attributs.
    
    This fixes various failures in the libiscsi testsuite.
    
    Personally I'd prefer to also remove the emulate_fua_write attribute
    as there is no good reason to disable it, but I'll leave that for
    a separate discussion.
    
    Signed-off-by: Christoph Hellwig <hch at lst.de>
    Signed-off-by: Nicholas Bellinger <nab at linux-iscsi.org>
    Signed-off-by: Andrei Vagin <avagin at openvz.org>
---
 drivers/target/target_core_device.c    | 30 +++++++-----------------------
 drivers/target/target_core_file.c      |  4 +---
 drivers/target/target_core_internal.h  |  2 ++
 drivers/target/target_core_sbc.c       |  5 +++--
 drivers/target/target_core_spc.c       | 12 ++++++++----
 drivers/target/target_core_transport.c | 19 +++++++++++++++++++
 include/target/target_core_base.h      |  6 ------
 7 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 81156de492a3..7dfe9641b640 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -794,16 +794,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_model_alias);
 
 int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
 {
-	if (flag != 0 && flag != 1) {
-		pr_err("Illegal value %d\n", flag);
-		return -EINVAL;
-	}
-
-	if (flag) {
-		pr_err("dpo_emulated not supported\n");
-		return -EINVAL;
-	}
-
+	printk_once(KERN_WARNING
+		"ignoring deprecated emulate_dpo attribute\n");
 	return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_dpo);
@@ -833,16 +825,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_fua_write);
 
 int se_dev_set_emulate_fua_read(struct se_device *dev, int flag)
 {
-	if (flag != 0 && flag != 1) {
-		pr_err("Illegal value %d\n", flag);
-		return -EINVAL;
-	}
-
-	if (flag) {
-		pr_err("ua read emulated not supported\n");
-		return -EINVAL;
-	}
-
+	printk_once(KERN_WARNING
+		"ignoring deprecated emulate_fua_read attribute\n");
 	return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_fua_read);
@@ -1547,9 +1531,9 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 
 	dev->dev_attrib.da_dev = dev;
 	dev->dev_attrib.emulate_model_alias = DA_EMULATE_MODEL_ALIAS;
-	dev->dev_attrib.emulate_dpo = DA_EMULATE_DPO;
-	dev->dev_attrib.emulate_fua_write = DA_EMULATE_FUA_WRITE;
-	dev->dev_attrib.emulate_fua_read = DA_EMULATE_FUA_READ;
+	dev->dev_attrib.emulate_dpo = 1;
+	dev->dev_attrib.emulate_fua_write = 1;
+	dev->dev_attrib.emulate_fua_read = 1;
 	dev->dev_attrib.emulate_write_cache = DA_EMULATE_WRITE_CACHE;
 	dev->dev_attrib.emulate_ua_intlck_ctrl = DA_EMULATE_UA_INTLLCK_CTRL;
 	dev->dev_attrib.emulate_tas = DA_EMULATE_TAS;
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 5c92a0814e24..4e87701792a8 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -726,9 +726,7 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 		 * for SCSI WRITEs with Forced Unit Access (FUA) set.
 		 * Allow this to happen independent of WCE=0 setting.
 		 */
-		if (ret > 0 &&
-		    dev->dev_attrib.emulate_fua_write > 0 &&
-		    (cmd->se_cmd_flags & SCF_FUA)) {
+		if (ret > 0 && (cmd->se_cmd_flags & SCF_FUA)) {
 			struct fd_dev *fd_dev = FD_DEV(dev);
 			loff_t start = cmd->t_task_lba *
 				dev->dev_attrib.block_size;
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 60381db90026..06f372c71a3d 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -78,6 +78,8 @@ int	transport_clear_lun_ref(struct se_lun *);
 void	transport_send_task_abort(struct se_cmd *);
 sense_reason_t	target_cmd_size_check(struct se_cmd *cmd, unsigned int size);
 void	target_qf_do_work(struct work_struct *work);
+bool	target_check_wce(struct se_device *dev);
+bool	target_check_fua(struct se_device *dev);
 
 /* target_core_stat.c */
 void	target_stat_setup_dev_default_groups(struct se_device *);
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 1448dce5fde8..5578e4d21226 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -714,14 +714,15 @@ static int
 sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
 {
 	if (cdb[1] & 0x10) {
-		if (!dev->dev_attrib.emulate_dpo) {
+		/* see explanation in spc_emulate_modesense */
+		if (!target_check_fua(dev)) {
 			pr_err("Got CDB: 0x%02x with DPO bit set, but device"
 			       " does not advertise support for DPO\n", cdb[0]);
 			return -EINVAL;
 		}
 	}
 	if (cdb[1] & 0x8) {
-		if (!dev->dev_attrib.emulate_fua_write || !se_dev_check_wce(dev)) {
+		if (!target_check_fua(dev)) {
 			pr_err("Got CDB: 0x%02x with FUA bit set, but device"
 			       " does not advertise support for FUA write\n",
 			       cdb[0]);
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 2dd2887a2598..1206ba77a4d3 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -477,7 +477,7 @@ spc_emulate_evpd_86(struct se_cmd *cmd, unsigned char *buf)
 	buf[5] = 0x07;
 
 	/* If WriteCache emulation is enabled, set V_SUP */
-	if (se_dev_check_wce(dev))
+	if (target_check_wce(dev))
 		buf[6] = 0x01;
 	/* If an LBA map is present set R_SUP */
 	spin_lock(&cmd->se_dev->t10_alua.lba_map_lock);
@@ -896,7 +896,7 @@ static int spc_modesense_caching(struct se_cmd *cmd, u8 pc, u8 *p)
 	if (pc == 1)
 		goto out;
 
-	if (se_dev_check_wce(dev))
+	if (target_check_wce(dev))
 		p[2] = 0x04; /* Write Cache Enable */
 	p[12] = 0x20; /* Disabled Read Ahead */
 
@@ -1008,8 +1008,12 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
 	     (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY)))
 		spc_modesense_write_protect(&buf[length], type);
 
-	if ((se_dev_check_wce(dev)) &&
-	    (dev->dev_attrib.emulate_fua_write > 0))
+	/*
+	 * SBC only allows us to enable FUA and DPO together.  Fortunately
+	 * DPO is explicitly specified as a hint, so a noop is a perfectly
+	 * valid implementation.
+	 */
+	if (target_check_fua(dev))
 		spc_modesense_dpofua(&buf[length], type);
 
 	++length;
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 8ae6748831f3..1a6ebd20e3bc 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2998,3 +2998,22 @@ int transport_generic_handle_tmr(
 	return 0;
 }
 EXPORT_SYMBOL(transport_generic_handle_tmr);
+
+bool
+target_check_wce(struct se_device *dev)
+{
+	bool wce = false;
+
+	if (dev->transport->get_write_cache)
+		wce = dev->transport->get_write_cache(dev);
+	else if (dev->dev_attrib.emulate_write_cache > 0)
+		wce = true;
+
+	return wce;
+}
+
+bool
+target_check_fua(struct se_device *dev)
+{
+	return target_check_wce(dev) && dev->dev_attrib.emulate_fua_write > 0;
+}
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 55c66cdbb414..3a7bbf7de29f 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -81,12 +81,6 @@
 #define DA_MAX_WRITE_SAME_LEN			0
 /* Use a model alias based on the configfs backend device name */
 #define DA_EMULATE_MODEL_ALIAS			0
-/* Emulation for Direct Page Out */
-#define DA_EMULATE_DPO				0
-/* Emulation for Forced Unit Access WRITEs */
-#define DA_EMULATE_FUA_WRITE			1
-/* Emulation for Forced Unit Access READs */
-#define DA_EMULATE_FUA_READ			0
 /* Emulation for WriteCache and SYNCHRONIZE_CACHE */
 #define DA_EMULATE_WRITE_CACHE			0
 /* Emulation for UNIT ATTENTION Interlock Control */


More information about the Devel mailing list