Re: [PATCH v2] lockd: refcount NLM_SHARE access/deny modes
Oscar Ou <oscarou-UelDjCVBxVpWk0Htik3J/[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Jul 01, 2026 at 09:40:46AM -0400, Chuck Lever wrote:
> 1. New thread per revision — no In-Reply-To: back to the
> old version.
> [...]
> 2. Version the subject tag, not RESEND. [PATCH v2],
> [PATCH v2 01/27] (lines 716–719, 726–731). RESEND is
> only for re-sending an /unchanged/ series that got no
> response (lines 379–380) — a modified series is never
> RESEND.
Understood, thanks -- v3 will go out as its own top-level
thread with a [PATCH v3] subject.
> With the per-value refcount a replayed SHARE instead increments the
> bucket a second time. Over UDP a lost reply followed by a client
> retransmit reaches nlmsvc_share_file() twice for one open, and if the
> client then sends one matching UNSHARE:
>
> SHARE(access=RW, deny=W) -> s_access_counts[3] = 1
> SHARE(access=RW, deny=W) (retransmit) -> s_access_counts[3] = 2
> UNSHARE(access=RW, deny=W) -> s_access_counts[3] = 1
>
> nlm_recompute_share() still sees s_access_counts[3] > 0, so s_access
> stays non-zero and the entry is never freed. Can this leave a stale
> share reservation that denies later conflicting opens until the host is
> torn down?
Yes. The inflated bucket only drains via the client's own
UNSHAREs or a FREE_ALL / SM_NOTIFY teardown, so until then
conflicting SHAREs from other owners see a spurious
NLM_LCK_DENIED.
> Here the client still holds open #2, but its reservation is gone, so a
> conflicting open from another owner would now be granted. Can a
> retransmitted UNSHARE release a grant the client still holds?
Agreed -- the UNSHARE retransmit should have been a no-op, but
v2 decrements a second time and releases a share the client
still holds.
> Taken together, is there a way to keep SHARE and UNSHARE idempotent
> under retransmission, some per-open identity to match on rather than a
> bare count, so a duplicated request lands on the same state the way the
> other NLM procedures do?
>
> One direction would be to track the set of held (access, deny)
> combinations rather than a count of each.
> [...]
> Setting or clearing a bit is idempotent, so a replayed SHARE or UNSHARE
> lands on the same mask, while distinct (access, deny) opens from one
> owner are still tracked separately.
Agreed -- that lines up with the invariant the other NLM handlers
rely on, and it's the shape I'll use for v3. Concretely:
#define LOCKD_FSH_BIT(a, d) BIT(((a) << 2) | (d))
SHARE: share->s_held_mask |= LOCKD_FSH_BIT(access, mode);
UNSHARE: share->s_held_mask &= ~LOCKD_FSH_BIT(access, mode);
s_access / s_mode are recomputed as the union of (i >> 2) / (i & 3)
over the set bits, and the entry is freed once s_held_mask is zero.
Both ops become an idempotent bit set/clear, so retransmits land
on the same state.
> The tradeoff is that two opens with the identical (access, deny) pair
> collapse to one bit, so a count of duplicate identical opens is lost.
> RFC 1813 defines SHARE and UNSHARE but does not require that repeated
> identical SHAREs from one owner survive an equal number of UNSHAREs
> [...]
> Does that match your reading?
Yes. RFC 1813 is silent on repeat semantics for SHARE / UNSHARE,
and section 4.5 explicitly treats non-idempotent requests as
something a reply cache has to protect, which lockd doesn't run
for NLM. So collapsing identical (access, deny) opens from one
owner into a single bit stays inside the protocol, and it buys
back the idempotency the other NLM handlers already assume.
> The bottom line is that after more consideration, IMO the reference
> count approach is not going to work, in particular because NLM still
> operates on unreliable network transports like UDP. Consider the
> above direction not as a mandate but as a possible approach that
> might address the issue you've reported while avoiding introducing
> non-idempotency.
Adopting the held-mask direction. v3 shortly, as a new top-level
thread.
Thanks,
Oscar
Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.