| Message ID | 1491873481-23900-4-git-send-email-mauricfo@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, 2017-04-10 at 22:18 -0300, Mauricio Faria de Oliveira wrote: > Currently, alua_rtpg() can change the 'state' and 'preferred' > values for the current port group _and_ of other port groups > present in the response buffer/descriptors. > > However, it reports such changes _only_ for the current port > group (i.e., only for 'pg' but not for 'tmp_pg'). > > This might cause uncertainty and confusion when going through > the kernel logs for analyzing/debugging scsi_dh_alua behavior, > which is not helpful during support and development scenarios. > > So, print such changes for other port groups than the current > one. > > This requires finding a scsi_device to call sdev_printk() with > for that other port group. Using 'tmp_pg->rtpg_sdev' is not an > option because in that 'if' conditional the 'tmp_pg' is not in > the ALUA_PG_RUNNING state, so that pointer may be NULL. So the > for-loop over the tmp->pg device_handler structures is used to > find a valid scsi_device that is associated to this port group. > > Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> > --- > drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c > index 0d3508f2e285..c2c9173fd883 100644 > --- a/drivers/scsi/device_handler/scsi_dh_alua.c > +++ b/drivers/scsi/device_handler/scsi_dh_alua.c > @@ -688,6 +688,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) > if ((tmp_pg == pg) || > !(tmp_pg->flags & ALUA_PG_RUNNING)) { > struct alua_dh_data *h; > + struct scsi_device *tmp_pg_sdev = NULL; > > tmp_pg->state = desc[0] & 0x0f; > tmp_pg->pref = desc[0] >> 7; > @@ -697,6 +698,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) > /* h->sdev should always be valid */ > BUG_ON(!h->sdev); > h->sdev->access_state = desc[0]; > + > + /* > + * If tmp_pg is not the running pg, and its worker > + * is not running, tmp_pg->rtpg_sdev might be NULL. > + * Use another/one associated scsi_device to print > + * its RTPG information. > + */ > + if ((tmp_pg != pg) && !tmp_pg_sdev) { > + tmp_pg_sdev = h->sdev; > + alua_rtpg_print(tmp_pg_sdev, tmp_pg, NULL); > + } > } > rcu_read_unlock(); > } Hello Mauricio, Please consider moving the alua_rtpg_print() call out of the loop. That will not only allow to eliminate the tmp_pg_sdev variable but will also reduce the nesting depth of the code. E.g. something like the following (untested) code: list_for_each_entry_rcu(h, &tmp_pg->dh_list, node) { [ ... ] } + h = list_first_or_null_rcu(&tmp_pg->dh_list, typeof(*h), node); + if (tmp_pg != pg && h) + alua_rtpg_print(h->sdev, tmp_pg, NULL); rcu_read_unlock(); Thanks, Bart.
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c index 0d3508f2e285..c2c9173fd883 100644 --- a/drivers/scsi/device_handler/scsi_dh_alua.c +++ b/drivers/scsi/device_handler/scsi_dh_alua.c @@ -688,6 +688,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) if ((tmp_pg == pg) || !(tmp_pg->flags & ALUA_PG_RUNNING)) { struct alua_dh_data *h; + struct scsi_device *tmp_pg_sdev = NULL; tmp_pg->state = desc[0] & 0x0f; tmp_pg->pref = desc[0] >> 7; @@ -697,6 +698,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg) /* h->sdev should always be valid */ BUG_ON(!h->sdev); h->sdev->access_state = desc[0]; + + /* + * If tmp_pg is not the running pg, and its worker + * is not running, tmp_pg->rtpg_sdev might be NULL. + * Use another/one associated scsi_device to print + * its RTPG information. + */ + if ((tmp_pg != pg) && !tmp_pg_sdev) { + tmp_pg_sdev = h->sdev; + alua_rtpg_print(tmp_pg_sdev, tmp_pg, NULL); + } } rcu_read_unlock(); }
Currently, alua_rtpg() can change the 'state' and 'preferred' values for the current port group _and_ of other port groups present in the response buffer/descriptors. However, it reports such changes _only_ for the current port group (i.e., only for 'pg' but not for 'tmp_pg'). This might cause uncertainty and confusion when going through the kernel logs for analyzing/debugging scsi_dh_alua behavior, which is not helpful during support and development scenarios. So, print such changes for other port groups than the current one. This requires finding a scsi_device to call sdev_printk() with for that other port group. Using 'tmp_pg->rtpg_sdev' is not an option because in that 'if' conditional the 'tmp_pg' is not in the ALUA_PG_RUNNING state, so that pointer may be NULL. So the for-loop over the tmp->pg device_handler structures is used to find a valid scsi_device that is associated to this port group. Signed-off-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> --- drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)