On 23/03/2026 00:08, Benjamin Marzinski wrote:
      k += off, desc += off) {
-               u16 group_id = get_unaligned_be16(&desc[2]);
-
-               spin_lock_irqsave(&port_group_lock, flags);
-               tmp_pg = alua_find_get_pg(pg->device_id_str, pg->device_id_len,
-                                         group_id);
-               spin_unlock_irqrestore(&port_group_lock, flags);
-               if (tmp_pg) {
-                       if (spin_trylock_irqsave(&tmp_pg->lock, flags)) {
-                               if ((tmp_pg == pg) ||
-                                   !(tmp_pg->flags & ALUA_PG_RUNNING)) {
-                                       struct alua_dh_data *h;
-
-                                       tmp_pg->state = desc[0] & 0x0f;
-                                       tmp_pg->pref = desc[0] >> 7;
-                                       rcu_read_lock();
-                                       list_for_each_entry_rcu(h,
-                                               &tmp_pg->dh_list, node) {
-                                               if (!h->sdev)
-                                                       continue;
-                                               h->sdev->access_state = desc[0];
-                                       }
-                                       rcu_read_unlock();
-                               }
-                               if (tmp_pg == pg)
-                                       tmp_pg->valid_states = desc[1];
-                               spin_unlock_irqrestore(&tmp_pg->lock, flags);
-                       }
-                       kref_put(&tmp_pg->kref, release_port_group);
+               u16 group_id_desc = get_unaligned_be16(&desc[2]);
+
+               spin_lock_irqsave(&h->lock, flags);
+               if (group_id_desc == group_id) {
+                       h->group_id = group_id;
+                       WRITE_ONCE(h->state, desc[0] & 0x0f);
+                       h->pref = desc[0] >> 7;
+                       WRITE_ONCE(sdev->access_state, desc[0]);
+                       h->valid_states = desc[1];
instead of alua_rtpg() updating the access_state all of the devices in
all the port groups, and the state and pref of all the port groups. It
now just sets these for one device. It seems like it's wasting a lot of
information that it used to use. For instance, now when a scsi command
returns a unit attention that the ALUA state has changed, it won't get
updated on all the devices, just the one that got the unit attention.

The fabric should then trigger this PG info update be re-scanned per-path/sdev (and not just a single sdev in the PG). From testing with a linux target, this is what happens - a UA is triggered per path when I changed the PG access state.


                }
+               spin_unlock_irqrestore(&h->lock, flags);
                off = 8 + (desc[7] * 4);
        }
skip_rtpg:
-       spin_lock_irqsave(&pg->lock, flags);
+       spin_lock_irqsave(&h->lock, flags);
        if (transitioning_sense)
-               pg->state = SCSI_ACCESS_STATE_TRANSITIONING;
+               h->state = SCSI_ACCESS_STATE_TRANSITIONING;

...

-
  static void alua_rtpg_work(struct work_struct *work)
  {
-       struct alua_port_group *pg =
-               container_of(work, struct alua_port_group, rtpg_work.work);
-       struct scsi_device *sdev, *prev_sdev = NULL;
+       struct alua_dh_data *h =
+               container_of(work, struct alua_dh_data, rtpg_work.work);
+       struct scsi_device *sdev = h->sdev;
        LIST_HEAD(qdata_list);
        int err = SCSI_DH_OK;
        struct alua_queue_data *qdata, *tmp;
-       struct alua_dh_data *h;
        unsigned long flags;
- spin_lock_irqsave(&pg->lock, flags);
-       sdev = pg->rtpg_sdev;
-       if (!sdev) {
-               WARN_ON(pg->flags & ALUA_PG_RUN_RTPG);
-               WARN_ON(pg->flags & ALUA_PG_RUN_STPG);
-               spin_unlock_irqrestore(&pg->lock, flags);
-               kref_put(&pg->kref, release_port_group);
-               return;
-       }
-       pg->flags |= ALUA_PG_RUNNING;
-       if (pg->flags & ALUA_PG_RUN_RTPG) {
-               int state = pg->state;
+       spin_lock_irqsave(&h->lock, flags);
+       h->flags |= ALUA_PG_RUNNING;
+       if (h->flags & ALUA_PG_RUN_RTPG) {
+               int state = h->state;
- pg->flags &= ~ALUA_PG_RUN_RTPG;
-               spin_unlock_irqrestore(&pg->lock, flags);
+               h->flags &= ~ALUA_PG_RUN_RTPG;
+               spin_unlock_irqrestore(&h->lock, flags);
                if (state == SCSI_ACCESS_STATE_TRANSITIONING) {
                        if (alua_tur(sdev) == SCSI_DH_RETRY) {
-                               spin_lock_irqsave(&pg->lock, flags);
-                               pg->flags &= ~ALUA_PG_RUNNING;
-                               pg->flags |= ALUA_PG_RUN_RTPG;
-                               if (!pg->interval)
-                                       pg->interval = ALUA_RTPG_RETRY_DELAY;
-                               spin_unlock_irqrestore(&pg->lock, flags);
-                               queue_delayed_work(kaluad_wq, &pg->rtpg_work,
-                                                  pg->interval * HZ);
+                               spin_lock_irqsave(&h->lock, flags);
+                               h->flags &= ~ALUA_PG_RUNNING;
+                               h->flags |= ALUA_PG_RUN_RTPG;
+                               if (!h->interval)
+                                       h->interval = ALUA_RTPG_RETRY_DELAY;
+                               spin_unlock_irqrestore(&h->lock, flags);
+                               queue_delayed_work(kaluad_wq, &h->rtpg_work,
+                                                  h->interval * HZ);
                                return;
                        }
                        /* Send RTPG on failure or if TUR indicates SUCCESS */
                }
-               err = alua_rtpg(sdev, pg);
-               spin_lock_irqsave(&pg->lock, flags);
+               err = alua_rtpg(sdev);
+               spin_lock_irqsave(&h->lock, flags);
- /* If RTPG failed on the current device, try using another */
-               if (err == SCSI_DH_RES_TEMP_UNAVAIL &&
-                   (prev_sdev = alua_rtpg_select_sdev(pg)))
-                       err = SCSI_DH_IMM_RETRY;
Previously, if the rtpg failed on a device, another device would be
tried, and the unusable device's alua state would get updated, along
with all the other device's states.

Where specifically are you referring to here please?

Now I don't see how a failed device
gets its state updated.

AFAICS, I am only not omitted how we iterate through the devices per-PG, as now we just do this work for all paths/scsi devices.

Thanks,
John

Reply via email to