Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afr_frame_return() should be called within the previous lock (without taking it again of course) #728

Closed
mykaul opened this issue Sep 19, 2019 · 3 comments

Comments

@mykaul
Copy link
Contributor

mykaul commented Sep 19, 2019

In several locations, we see the following code:

   }
    UNLOCK(&frame->lock);

    call_count = afr_frame_return(frame);

Which essentially frees the frame lock that afr_frame_return() would take immediately after, just to decrement the call count. Instead, just perform --local->call_count; inside that lock.

Functions that I've seen this:
afr_flush_cbk
afr_fsyncdir_cbk
afr_statfs_cbk
afr_opendir_cbk
__afr_dir_write_cbk
__afr_inode_write_cbk
afr_open_cbk

@mykaul
Copy link
Contributor Author

mykaul commented Oct 1, 2019

gluster-ant pushed a commit that referenced this issue Oct 7, 2019
If you are already under lock, just decrement the call count
directly instead of removing the lock, re-taking the lock
and decrementing.

Implements #728
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

Change-Id: I3fa20b4651fbdb826655c5a03baeed46e99b5487
amarts pushed a commit to amarts/glusterfs that referenced this issue Oct 9, 2019
If you are already under lock, just decrement the call count
directly instead of removing the lock, re-taking the lock
and decrementing.

Implements gluster/glusterfs#728
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

Change-Id: I3fa20b4651fbdb826655c5a03baeed46e99b5487
@mykaul
Copy link
Contributor Author

mykaul commented Oct 19, 2019

@mykaul mykaul closed this as completed Oct 19, 2019
@mykaul
Copy link
Contributor Author

mykaul commented Jan 20, 2020

@amarts haven't given up on this for DHT ( https://review.gluster.org/#/c/glusterfs/+/23458/.) and will try to get it in to 8.0

amarts pushed a commit to amarts/glusterfs that referenced this issue Mar 24, 2020
If you are already under lock, just decrement the call count
directly instead of removing the lock, re-taking the lock
and decrementing.

Implements gluster/glusterfs#728
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>

Change-Id: I3fa20b4651fbdb826655c5a03baeed46e99b5487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant