On 10/14/20 10:27 PM, Muneendra wrote:
Added a store functionality to set rport port_state using sysfs
under fc_remote_ports/rport-*/port_state
With this functionality the user can move the port_state from
Marginal->Online and Online->Marginal.
On Marginal :This interface will set SCMD_NORETRIES_ABORT bit in
scmd->state for all the io's on the scsi device associated
with remote port.
On Online :This interface will clear SCMD_NORETRIES_ABORT bit in
scmd->state for all the pending io's on the scsi device associated
with remote port.
Below is the interface provided to set the port state to Marginal
and Online.
echo "Marginal" >> /sys/class/fc_remote_ports/rport-X\:Y-Z/port_state
echo "Online" >> /sys/class/fc_remote_ports/rport-X\:Y-Z/port_state
Signed-off-by: Muneendra <muneendra.kumar@xxxxxxxxxxxx>
---
v3:
Removed the port_state from starget attributes.
Enabled the store functionality for port_state under remote port.
used the starget_for_each_device to traverse around all the devices
under rport
v2:
Changed from a noretries_abort attribute under fc_transport/target*/ to
port_state for changing the port_state to a marginal state
---
drivers/scsi/scsi_transport_fc.c | 85 +++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index df66a51d62e6..ac5283b645a6 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -943,7 +943,88 @@ show_fc_rport_roles (struct device *dev, struct device_attribute *attr,
static FC_DEVICE_ATTR(rport, roles, S_IRUGO,
show_fc_rport_roles, NULL);
-fc_private_rport_rd_enum_attr(port_state, FC_PORTSTATE_MAX_NAMELEN);
+static void
+device_chg_noretries_abort(struct scsi_device *sdev, void *data)
+{
+ scsi_chg_noretries_abort_io_device(sdev, *(bool *)data);
+}
+
+static int
+target_chg_noretries_abort(struct device *dev, void *data)
+{
+ if (scsi_is_target_device(dev))
+ starget_for_each_device(to_scsi_target(dev), data,
+ device_chg_noretries_abort);
+ return 0;
+}
+
+static void scsi_target_chg_noretries_abort(struct device *dev, bool set)
+{
+ if (scsi_is_target_device(dev))
+ starget_for_each_device(to_scsi_target(dev), &set,
+ device_chg_noretries_abort);
+ else
+ device_for_each_child(dev, &set, target_chg_noretries_abort);
+}
+
+/*
+ * Sets port_state to Marginal/Online.
+ * On Marginal it Sets no retries on abort in scmd->state for all
+ * outstanding io of all the scsi_devs
+ * This only allows ONLINE->MARGINAL and MARGINAL->ONLINE
The above comments are not needed since not counting comments the code
is almost the same number of lines as the comments. Plus the comments
below say the same thing. And the functions/fields/variables you are
using below are pretty clear in what they are doing.
+ */
+static ssize_t fc_rport_set_marginal_state(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct fc_rport *rport = transport_class_to_rport(dev);
+ enum fc_port_state port_state;
+ int ret = 0;
+
+ ret = get_fc_port_state_match(buf, &port_state);
The kernel test robot mentioned this.
+
+ if (port_state == FC_PORTSTATE_MARGINAL) {
+ /*
+ * Change the state to marginal only if the
+ * current rport state is Online
+ * Allow only Online->marginal
+ */
+ if (rport->port_state == FC_PORTSTATE_ONLINE) {
+ rport->port_state = port_state;
+ scsi_target_chg_noretries_abort(&rport->dev, 1);
+ }
Should this return a failure if port_state is not online?
+ } else if (port_state == FC_PORTSTATE_ONLINE) {
+ /*
+ * Change the state to Online only if the
+ * current rport state is Marginal
+ * Allow only MArginal->Online
+ */
+ if (rport->port_state == FC_PORTSTATE_MARGINAL) {
+ rport->port_state = port_state;
+ scsi_target_chg_noretries_abort(&rport->dev, 0);
+ }
Same here.
+ } else
+ return -EINVAL;
+ return count;
+}
+
+static ssize_t
+show_fc_rport_port_state(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ const char *name;
+ struct fc_rport *rport = transport_class_to_rport(dev);
+
+ name = get_fc_port_state_name(rport->port_state);
+ if (!name)
+ return -EINVAL;
+
+ return snprintf(buf, 20, "%s\n", name);
+}
+
+static FC_DEVICE_ATTR(rport, port_state, 0444 | 0200,
+ show_fc_rport_port_state, fc_rport_set_marginal_state);
+
fc_private_rport_rd_attr(scsi_target_id, "%d\n", 20);
/*
@@ -2266,7 +2347,7 @@ fc_attach_transport(struct fc_function_template *ft)
SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_name);
SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_id);
SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(roles);
- SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_state);
+ SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(port_state);
SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(scsi_target_id);
SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(fast_io_fail_tmo);
