-
-
usr/src/uts/common/fs/xattr.c (Diff revision 1) Can you please explain what's going on here?
How do we end up with an extra ref?How do we know it's OK to call VN_RELE_LOCKD here?
(which is only OK if dvp->v_count > 1 right?
at least comparing with what vn_rele does)
8805 xattr_dir_lookup() can leak a vnode hold
Review Request #788 — Created Dec. 4, 2017 and submitted
Information | |
---|---|
vgusev | |
illumos-gate | |
master | |
8805 | |
d15e368... | |
Reviewers | |
general | |
gwr |
8805 xattr_dir_lookup() can leak a vnode hold
After steps described at #8805, dataset is umounted successfully, i.e. no "busy".
-
-
usr/src/uts/common/fs/xattr.c (Diff revision 1) Yes, sure.
Here is allocation and initialization with
pre-alloc-check
allocation
post-alloc-check.-
dvp->v_xattrdir is checked first time on the line 1614 under held lock.
-
gfs_dir_create() is called w/o lock (i.e. allocation).
-
dvp->v_xattrdir is checked again and set on the line 1691 (orig) under held dvp->v_lock, i.e. post alloc check. And thic check introduces two paths: winner and looser.
As described in comments for case "dvp->v_xattr != NULL":
/* * We lost the race to create the xattr dir. * Destroy this one, use the winner. We can't * just call VN_RELE(*vpp), because the vnode * is only partially initialized. */
and the code below comment is doing manual freeing.
Note, dvp->v_count >= 2 before line 1674 (new) because:
a. winner has got the reference already in gfs_dir_create()
b. this check is under held lock
c. looser also got the reference in gfs_dir_create()d. reference is released safely under held lock in gfs_file_release()
Note-2, winner is: lines 1690-1691 (orig)
-
-
-
usr/src/uts/common/fs/xattr.c (Diff revision 1) Maybe minor nit:
Strictly speaking, this comment is not needed here and might be confusing a bit (we do not have similar comment at line 1615).
I suggest either to remove it, or replace it with something like:
/* * Use the winner */
But if you do not wish do to do, I'm okay with that.
-
-
usr/src/uts/common/fs/xattr.c (Diff revision 1) At least for me, it would have helped a lot (when I last worked on this code, rather long ago) if there were a more helpful comment here about what vnode holds come from where in both the winner and loser code paths, and what happens to those holds (returned to caller, released).
Change Summary:
Changed some comments
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+3 -7) |