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

shrink buf changed behavior #335

Open
ofirvin opened this issue Jan 29, 2025 · 3 comments
Open

shrink buf changed behavior #335

ofirvin opened this issue Jan 29, 2025 · 3 comments

Comments

@ofirvin
Copy link

ofirvin commented Jan 29, 2025

Hi,
We have been going over a few recent commits in libntirpc.
and we saw 1352b71
and it kind of looks like chunk_unref_locked can do a double unlock. we are not very familiar with that layer, but do you think that can happen? and if so is that ok?

@dang
Copy link
Collaborator

dang commented Jan 29, 2025

I'm confused, there appear to be no unlocks in chunk_unref_locked(). Did you mean a different function?

@ofirvin
Copy link
Author

ofirvin commented Feb 2, 2025

chunk_unref_locked calls get_lru_chunk_with_lock. hich sometimes leaves ioq unlocked.

and then do_shrink can unlock ioq again if !is_same_buflist(io_buf, new_io_buf, rdma_xprt)

@dang
Copy link
Collaborator

dang commented Feb 3, 2025

I think it's safe. get_lru_chunk_with_lock() only unlocks ioqh->qmutex if pthread_mutex_trylock() succeeded, which means that ioq was not already locked, and we succeeded in taking the lock. The correct thing to do then is unlock it. If trylock() fails (we didn't get the lock, it was already locked, potentially by us), then we skip to the next buffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants