<html><body>Hello,<div><br></div><div>I'm a little bit confused from your answer. If I understood it correctly Jakub is asking what would be the best practice. Of course in our assignment we should know our code perfectly. But in real life that's not the case. And I thought by having teams you're trying to get at least a little bit closer to reality and teach us best practices which are useful when we program something bigger. Because when we approach our assignment with attitude "the code isn't that big", we can scratch most of the principles of writing good code which would only pay off in the long term.</div><div><br></div><div>We are encouraged to use defensive mechanisms in our code (at least I thought so from the feedback) and check things which aren't strictly necessary to check but help to avoid making a mistake or at least ease the debugging process. We are taught that we should write code as though someone else will continue development. And if I go really carefully through my code and reorder a function in a way that the lock (interrupt disable) isn't necessary someone else may come to my program do a small tweak and break it because he didn't know this is possibly a critical section because it wasn't locked.</div><div><br></div><div>With this in mind, is it still true that we should disable interrupts as little as possible? Is it better to leave a comment "This doesn't need a lock"? Is it better to put the lock there with comment "This lock isn't necessary but better safe than sorry"? There may be cases where analysis whether a section does need a lock may be complicated and susceptible to errors when edited/refactored/changed. Should I put a lock there? Maybe comment "Shouldn't be necessary but just in case".</div><div><br></div><div><span style="background-color:transparent">For example sem_destroy(sem_t* sem) which looks like this.</span></div><div><span style="background-color:transparent">```<br><div> enter_critical_section();</div><div><br></div><div> assert(sem != NULL);</div><div> sem_check_valid(sem);</div><div><br></div><div> panic_if(!queue_is_empty(&sem->thread_queue), "Cannot destroy semaphore with threads waiting");</div><div> sem->magic = MAGIC_INVALID;</div><div><br></div><div> leave_critical_section();</div><div>```<br><br></div></span></div><div> I feel this the most readable form. However not the most effective. I like that all validity checks (NULL assert, sem_check_valid) are together because the feel like one logic block "Check if everything is all right" (Maybe we should put NULL assert into sem_check_valid as well? However then we cant put enter_critical_section() in the middle). But theoretically assert could be outside the critical section. The same goes for sem_check_valid(). It could be put outside critical section as long as destroy isn't called twice. And because in our assignment we know the code well, we can make sure this doesn't happen. However if somebody else used our semaphore he could be confused that sometimes double destroy prints error and sometimes succeeds without problems. Even though it would be explicitly stated in documentation that double destroy is undefined behavior. However this feels just wrong. </div><div><br></div><div>And let's say I decide that most of the function need to be critical section from top to bottom and then one function doesn't need from top to bottom but only part of it. However the code is much more readable when the pattern goes on and I put that one function in critical section top to bottom as well. Because then someone else might do stupid mistake when he presumes that all functions are critical section top to bottom but didn't notice this one weird exception.</div><div><br></div><div>Therefore I don't feel like instructions "use it sparingly" and "use it for minimum duration possible" are complete. I'm trying to avoid the same problem we had with a02 when we got minus points for good code practices we didn't know about and which weren't talked about in this course. Therefore I'm trying to ask in advance here what are the best practices with locking because I have no idea what's right. I'd appreciate if you could go through our code and say if we use these right or wrong because in our team we don't know. Or better provide some guidelines so we do it right and don't loose point unnecessarily.</div><div><span style="background-color:transparent"><br class="-wm-Apple-interchange-newline">And of course I'm not asking only for this case.</span><span style="background-color:transparent"> This happens in higher languages. How different is the case here and in a higher-level language? Are there some rules which apply to synchronization practices in general?</span><br></div><div><span style="background-color:transparent"><br></span></div><div><span style="background-color:transparent">Thank you for your time and answers</span></div><div><span style="background-color:transparent">Martin ZImen</span></div><aside><br></aside><aside>
---------- Původnà e-mail ----------<br>
Od: Petr Tuma <petr.tuma@d3s.mff.cuni.cz><br>
Komu: Operating Systems Course <nswi004@d3s.mff.cuni.cz>, Jakub Pelc <jakub.pelc@email.cz><br>
Datum: 29. 11. 2020 17:06:41<br>
PÅedmÄt: Re: [NSWI004] a04 no-interrupt best practices
</aside><br><blockquote data-email="petr.tuma@d3s.mff.cuni.cz">Hello Jakub,<br><br>I think you are in fact answering yourself by the very act of asking - it is not a good sign if you are not sure what can be going on in your code :-) that said, there are certainly systems that are so complex one cannot be sure, only we kind of hoped your assignments will not grow into that size :-)<br><br> From the OS implementation perspective, disabling interrupts is an operation that should be used sparingly, and for minimum duration possible. In a real system it does not avert most problems because it is processor local.<br><br>Petr<br><br><br>On 29/11/2020 15:21, Jakub Pelc wrote:<br>> Hello,<br>> <br>> I've encountered a few (short) functions where I think switching threads at any point should not break anything, but I'm not 100% sure, so I disabled interrupts in this code. Is this the right approach? What are the best practices with this kind of "defensive" synchronization?<br>> <br>> Best regards,<br>> Jakub Pelc<br>> <br>> _______________________________________________<br>> NSWI004 mailing list<br>> NSWI004@d3s.mff.cuni.cz<br>> https://d3s.mff.cuni.cz/mailman/listinfo/nswi004<br>> <br>_______________________________________________<br>NSWI004 mailing list<br>NSWI004@d3s.mff.cuni.cz<br>https://d3s.mff.cuni.cz/mailman/listinfo/nswi004<br></blockquote></body></html>