2008/12/13 Gerrit Renker <gerrit@erg.abdn.ac.uk>:
> | > +static int ccid_request_module(u8 id)
> | > +{
> | > +       if (!in_atomic()) {
> | > +               ccids_read_lock();
> | > +               if (ccids[id] == NULL) {
> | > +                       ccids_read_unlock();
> | > +                       return request_module("net-dccp-ccid-%d", id);
> | > +               }
> | > +               ccids_read_unlock();
> | > +       }
> | > +       return 0;
> | > +}
> |
> | Just a random thought: does this lock really do anything useful here?
> |
> Reading the (shared) 'ccids' array is the solution chosen to check whether
> the module for CCID with number 'id' is already loaded.
>
> It would be bad to call request_module() each time a new DCCP socket is
> opened. Using the 'ccids' array may not be the only way to check whether
> a given module (whose name depends on the value of 'id') is loaded.
>
> But if this solution is chosen, then it requires to protect the read
> access to 'ccids', which is shared among all DCCP sockets.

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;
}

Best Regards,
Michał Mirosław