[Asterisk-code-review] func_lock: fix multiple-channel-grant problems. (asterisk[master])

[Jaco Kroon]

Attention is currently required from: Sean Bright.
Jaco Kroon has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15227 )

Change subject: func_lock: fix multiple-channel-grant problems.
......................................................................


Patch Set 3:

(2 comments)

Patchset:

PS3: 
> re: volatile […]
unloading is only ever set once.  And will always be async to the reads thereof.  And it's absolutely critical that if an unload is started that wherever we get to in get_lock() we honour that, so if there is any chance that the compile will cache the unloading variable in a register it should be eliminated.  volatile is the easy way of ensuring that, without locking and memory barriers (much like the jiffies example in the document you referenced).

As a general rule, yes, I agree that use of volatile must be avoided.  Looking at get_lock again for this, the only real risk is the last conditional for ast_cond_signal in my opinion.  I think it should be safe to drop volatile here since there all access to unloading prior to this are conditional anyway (and result in return), so unloading is extremely unlikely to be consider register cacheable anyway.  If you prefer I'll drop it, since I never unload this module except for shutdown anyway a crash at that point won't bother me too much but I'd still prefer to avoid it.

Your call.


PS3: 
Thanks!  Anything else?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15227
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I6f68b5ec82ff25b2909daf6e4d19ca864a463e29
Gerrit-Change-Number: 15227
Gerrit-PatchSet: 3
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Friendly Automation
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-CC: Sean Bright <sean.bright at gmail.com>
Gerrit-Attention: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Fri, 18 Dec 2020 20:04:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20201218/28d6b565/attachment.html>