Skip to content

Commit 3dcd1d8

Browse files
J. Bruce Fieldschucklever
authored andcommitted
nfsd: improve stateid access bitmask documentation
The use of the bitmaps is confusing. Add a cross-reference to make it easier to find the existing comment. Add an updated reference with URL to make it quicker to look up. And a bit more editorializing about the value of this. Signed-off-by: J. Bruce Fields <bfields@redhat.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
1 parent 70e94d7 commit 3dcd1d8

2 files changed

Lines changed: 14 additions & 4 deletions

File tree

fs/nfsd/nfs4state.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,18 +360,24 @@ static const struct nfsd4_callback_ops nfsd4_cb_notify_lock_ops = {
360360
* st_{access,deny}_bmap field of the stateid, in order to track not
361361
* only what share bits are currently in force, but also what
362362
* combinations of share bits previous opens have used. This allows us
363-
* to enforce the recommendation of rfc 3530 14.2.19 that the server
364-
* return an error if the client attempt to downgrade to a combination
365-
* of share bits not explicable by closing some of its previous opens.
363+
* to enforce the recommendation in
364+
* https://datatracker.ietf.org/doc/html/rfc7530#section-16.19.4 that
365+
* the server return an error if the client attempt to downgrade to a
366+
* combination of share bits not explicable by closing some of its
367+
* previous opens.
366368
*
367-
* XXX: This enforcement is actually incomplete, since we don't keep
369+
* This enforcement is arguably incomplete, since we don't keep
368370
* track of access/deny bit combinations; so, e.g., we allow:
369371
*
370372
* OPEN allow read, deny write
371373
* OPEN allow both, deny none
372374
* DOWNGRADE allow read, deny none
373375
*
374376
* which we should reject.
377+
*
378+
* But you could also argue that our current code is already overkill,
379+
* since it only exists to return NFS4ERR_INVAL on incorrect client
380+
* behavior.
375381
*/
376382
static unsigned int
377383
bmap_to_share_mode(unsigned long bmap)

fs/nfsd/state.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,10 @@ struct nfs4_ol_stateid {
568568
struct list_head st_locks;
569569
struct nfs4_stateowner *st_stateowner;
570570
struct nfs4_clnt_odstate *st_clnt_odstate;
571+
/*
572+
* These bitmasks use 3 separate bits for READ, ALLOW, and BOTH; see the
573+
* comment above bmap_to_share_mode() for explanation:
574+
*/
571575
unsigned char st_access_bmap;
572576
unsigned char st_deny_bmap;
573577
struct nfs4_ol_stateid *st_openstp;

0 commit comments

Comments
 (0)