Conversation
Changing filename while inactive did not re-check thin provisioning, so auto thin_provisioned could stick after swapping backends. Mark reexam_pending so the next activate reuses the existing reexamine path without affecting ALUA-only activate cycles. Co-authored-by: Cursor <cursoragent@cursor.com>
lnocturno
added a commit
that referenced
this pull request
Sep 17, 2026
Changing filename while a vdisk is inactive leaves the old backend's thin-provisioning capabilities cached. For example, switching between a plain device and a discard-capable cache can leave automatic thin provisioning disabled or enabled for the wrong backend. Mark only a TP recheck as pending after a successful filename write. Consume it in the common activation path used by active=1 and ALUA. Do not request a full backend reexamination for an existing device: that would also overwrite the exported size and recheck cache mode. The initial deferred reexamination still performs its normal work; if it completes the TP check, activation does not repeat that check. Make the TP probe safe to call repeatedly: - Track successful creation of gen_tp_soft_threshold_reached_UA with tp_ua_attr_created. This avoids duplicate sysfs creation for both thin-to-thin switches and thin-to-non-thin-to-thin round trips. Keep the attribute until device removal; its existing store callback rejects requests while TP is disabled. Failed creation leaves the flag clear so a later probe can retry. - Compare the previous effective TP state and advertised UNMAP limits with vdisk_tp_changed(). Notify on TP enable/disable and on changed granularity, alignment, maximum LBA count or discard-zeroes behavior while TP remains enabled. Ignore stale limits while TP is disabled. - Keep the requested manual TP setting separate from the effective backend capability. An explicit thin_provisioned=0 stays disabled. An explicit thin_provisioned=1 is temporarily disabled on a backend without TP and restored when a later backend supports it. Reusing the effective value as the request would lose that setting on a thin-to-non-thin-to-thin round trip. - On a probe-open error, leave the previous capabilities and pending flag intact. Activation closes any reopened descriptors and returns the device to inactive state so a subsequent activation can retry. Filename replacement, explicit activation and ALUA activation already serialize through scst_alua_lock(). Use that same lock for the new state so another filename write cannot race with consuming the pending check. Keep the new bool fields outside the command-side bitfields to avoid sharing their read-modify-write storage with command callbacks. Queue INQUIRY DATA HAS CHANGED through vdev_inq_changed_work. Calling scst_dev_inquiry_data_changed() directly from activation could recurse on scst_mutex. The existing device-free callback cancels this work before releasing the device. Ordinary ALUA or active cycles without a filename write do not request another TP probe or notification. Link: #389
Contributor
|
Thanks for reporting this and proposing a fix. I prepared an alternative in #391 Setting reexam_pending also triggers size and cache-mode reexamination. The alternative limits the refresh to thin-provisioning capabilities, handles repeated sysfs attribute creation, and notifies initiators when the advertised capabilities change. It also preserves explicit thin_provisioned settings across backend changes. Could you review it and, if possible, check whether it resolves your original scenario? If it does, I'd like to merge that version and close this PR as superseded. Gleb |
Author
|
Thanks for the alternative. I reviewed and tested #391, and I can confirm that it fixes my original scenario. The behavior now works as expected, including the thin-provisioning capability refresh and preserving the explicit From my testing, this version resolves the issue I originally reported, so I’m happy with merging #391 and closing the current PR as superseded. |
lnocturno
added a commit
that referenced
this pull request
Sep 28, 2026
Changing filename while a vdisk is inactive leaves the old backend's thin-provisioning capabilities cached. For example, switching between a plain device and a discard-capable cache can leave automatic thin provisioning disabled or enabled for the wrong backend. Mark only a TP recheck as pending after a successful filename write. Consume it in the common activation path used by active=1 and ALUA. Do not request a full backend reexamination for an existing device: that would also overwrite the exported size and recheck cache mode. The initial deferred reexamination still performs its normal work; if it completes the TP check, activation does not repeat that check. Make the TP probe safe to call repeatedly: - Track successful creation of gen_tp_soft_threshold_reached_UA with tp_ua_attr_created. This avoids duplicate sysfs creation for both thin-to-thin switches and thin-to-non-thin-to-thin round trips. Keep the attribute until device removal; its existing store callback rejects requests while TP is disabled. Failed creation leaves the flag clear so a later probe can retry. - Compare the previous effective TP state and advertised UNMAP limits with vdisk_tp_changed(). Notify on TP enable/disable and on changed granularity, alignment, maximum LBA count or discard-zeroes behavior while TP remains enabled. Ignore stale limits while TP is disabled. - Keep the requested manual TP setting separate from the effective backend capability. An explicit thin_provisioned=0 stays disabled. An explicit thin_provisioned=1 is temporarily disabled on a backend without TP and restored when a later backend supports it. Reusing the effective value as the request would lose that setting on a thin-to-non-thin-to-thin round trip. - On a probe-open error, leave the previous capabilities and pending flag intact. Activation closes any reopened descriptors and returns the device to inactive state so a subsequent activation can retry. Filename replacement, explicit activation and ALUA activation already serialize through scst_alua_lock(). Use that same lock for the new state so another filename write cannot race with consuming the pending check. Keep the new bool fields outside the command-side bitfields to avoid sharing their read-modify-write storage with command callbacks. Queue INQUIRY DATA HAS CHANGED through vdev_inq_changed_work. Calling scst_dev_inquiry_data_changed() directly from activation could recurse on scst_mutex. The existing device-free callback cancels this work before releasing the device. Ordinary ALUA or active cycles without a filename write do not request another TP probe or notification. Link: #389
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Gleb,
We hit an interesting case where, after swapping a vdisk backend filename (e.g. OpenCAS in/out), auto thin_provisioned did not follow the new backend, so we had to clear or set that flag by hand on
every swap. It looks like this can be handled in code so a filename change re-checks thin provisioning on the next activate and we do not need to adjust the flag manually each time.
Summary
• Changing a vdisk filename while the device is inactive (e.g. active=0 / transitioning) did not re-check backend thin-provisioning support.
• After pointing at a discard-capable backend (e.g. OpenCAS), auto thin_provisioned became 1. After switching filename back to the original device, it could stay 1 even when that backend does not
support thin provisioning (and the reverse stuck case as well).
• On a successful inactive filename change, set reexam_pending so the next activate reuses the existing vdisk_reexamine() path (size / flush / thin provisioning). Pure ALUA/active cycles that do
not change filename are unchanged.
Test plan
◻ Export a non-thin backend (thin_provisioned = 0).
◻ Set inactive, change filename to a discard-capable device (e.g. OpenCAS), activate → thin_provisioned becomes 1.
◻ Set inactive, change filename back to the original, activate → thin_provisioned returns to 0.
◻ Confirm an ALUA/active flip without a filename change does not force an extra reexamine beyond existing behavior.