2008/12/16 Gerrit Renker <gerrit@xxxxxxxxxxxxxx>:> | Since the lock is dropped after checking ccids[id] then there's> | a window where multiple request_module()s can be called if multiple> | applications create a DCCP socket at a same time. The code below> | should do the same without a lock (ccids is a static array,> | so ccids[N] is always at the same place).> |> | static int ccid_request_module(u8 id)> | {> | if (!in_atomic()) {> | rmb();> | if (ccids[id] == NULL)> | return request_module("net-dccp-ccid-%d", id);> | }> | return 0;> | }> |> Sorry Michael, but this is really just a "random thought". What you are> in effect saying is that reader/writer locks can be replaced with just a> read memory barrier.
> Please have a more detailed look at net/dccp/ccid.c. I also checked how> other subsystems handle comparable situations of module loading: the> implementation details differ, but the principle is the same: there are> mutexes, semaphores, and spinlocks in use to protect those shared> structures that are related to the loaded module.>> Hence your suggestion does not improve the code. I maintain that it is> correct. And it has proven to work in the test tree for more than one> year, including tests with up to 100 parallel (iperf) connections.
The read-lock is just a memory barrier here (not a read memory barrieras I wrotebefore) and it's not needed here. If I read the code correctly you aretesting a singlepointer to be NULL and don't really care about ordering wrt moduleinitialization.
You can actually annotate ccids[] as read_mostly (it's changed only on moduleload/unload) and protect it with RCU instead of the home-grown rwlockyou are using.
Best Regards,Michał Mirosław˙ôčş{.nÇ+?ˇ?Ž??+%?Ë˙ąéÝśĽ?w˙ş{.nÇ+?ˇ?qĘ˙?{ayş Ę?Ú?ë,j˘fŁ˘ˇh??ď?ę˙?ęçz_čŽ(é???ݢj"?úśm§˙˙žŤţGŤ?é˙˘¸??¨čÚ&Łř§~?á
