[gnutls-devel] GnuTLS | priority: rework config reloading logic and locking (!1483)

[Read-only notification of GnuTLS library development activities]

Alexander Sosedkin commented on a discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_721296096

>  			 * allow it to be updated without restarting
>  			 * all applications
>  			 */
> +			GNUTLS_STATIC_MUTEX_LOCK(system_wide_config_mutex);
>  			_gnutls_update_system_priorities();

I hope that all the heavy lifting here is localized in `_gnutls_update_system_priority` and is conditioned on actually reading the file after failing the mtime stat check; and then everyone has to wait no matter what. Provided no actual reload is necessary, it'll be just a stat call we wrlock for, and the rest shouldn't really meaningfully be long, just a bit of syscall-free string crunching, right? So, I see little benefit in scenario 2 over 1: we unconditionally wrlock in both and the scope of wrlocking isn't much different.


Now, one could keep advancing in your suggested direction and narrow wrlocking further to also exclude the mtime check:

```
// rdlock
inside _gnutls_update_system_priorities {
    if (/* mtime_check() */)
        return;
    // upgrade rdlock to wrlock
    // init check
    // heavy lifting
    // unlock
}

do {
    // rdlock
    // access
    // unlock
}
```

that allows for some executions to attempt no wrlocks, but will the gains between 1 and 3 be worth the complexity? I doubt it. If it were on me, I'd probably leave a note saying `TODO if somebody complains about the performance: locking could be narrowed to wrlocking for the part past the mcheck` and move on for now =)

-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_721296096
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20211102/4fcc949f/attachment-0001.html>