CODEWITHSUNDEEP
multithreadinglinux-kernel
Jacob
Jacob

Reputation: 80

Correct interactions between Linux kernel wait-queues and lists

I'm writing a Linux kernel module which involves a list being read/written from different process contexts and feel I'm missing functionality equivalent to pthread_cond_wait() and co. from user-space.

Naively I might write something like this:

static LIST_HEAD(request_list);
static DEFINE_MUTEX(request_list_mutex);
static DECLARE_WAIT_QUEUE_HEAD(request_list_post_wq);

static void post_request(request_t *request)
{
    mutex_lock(request_list_mutex);
    list_add(request, request_list);
    mutex_unlock(request_list_mutex);
    wake_event(request_list_post_wq);
}

static void wait_and_consume_request()
{
    mutex_lock(request_list_mutex);
    if(list_empty(request_list)) {
        mutex_unlock(request_list_mutex);
        wait_event(request_list_post_wq, !list_empty(request_list));
        mutex_lock(request_list_mutex);
    }

    // do something with request

    mutex_unlock(request_list_mutex);
}

However, this looks like it will have a race condition in the consumer function between waking on a non-empty list and then re-acquiring the mutex if there are multiple consumers. At the same time I have to release the mutex before waiting otherwise nothing will ever be able to add to the list.

I considered writing a function which locks the request_list, and only unlocks it if it's still empty and use this as the conditional to wait_event... but googling around I've seen lots of examples of people writing wait_event(...., !list_empty(...)) so I must be missing something?

Upvotes: 3

Views: 838

Answers (1)

JohnFlux
JohnFlux

Reputation: 366

The helper function that the other person suggested isn't needed at all:

static int list_is_not_empty()
{
    int rv = 1;
    mutex_lock(request_list_mutex);
    rv = !list_empty(request_list);
    mutex_unlock(request_list_mutex);
    return rv;
}

There's no need to lock the list just to see if it's empty or not. So simply:

static void wait_and_consume_request()
{
    wait_event(request_list_post_wq, !list_empty(request_list));
    mutex_lock(request_list_mutex);
    if(!list_empty(request_list)) {
        // do something with request
    }
    mutex_unlock(request_list_mutex);
}

But this won't guarantee that you actually consume a request. If we do want to ensure that we consume exactly one request, then:

static void wait_and_consume_request()
{
    mutex_lock(request_list_mutex);            
    while(list_empty(request_list)) {
        mutex_unlock(request_list_mutex);
        wait_event(request_list_post_wq, !list_empty());
        lock_mutex();
     }
     // do something with request
     mutex_unlock(request_list_mutex);
}

Here's a real example from the kernel in drivers/misc/carma/carma-fpga.c (I just took the first example that I could see)

      spin_lock_irq(&priv->lock);

      /* Block until there is at least one buffer on the used list */
      while (list_empty(used)) {
              spin_unlock_irq(&priv->lock);

              if (filp->f_flags & O_NONBLOCK)
                      return -EAGAIN;

              ret = wait_event_interruptible(priv->wait, !list_empty(used));
              if (ret)
                      return ret;

              spin_lock_irq(&priv->lock);
      }

      /* Grab the first buffer off of the used list */
      dbuf = list_first_entry(used, struct data_buf, entry);
      list_del_init(&dbuf->entry);

      spin_unlock_irq(&priv->lock);

Upvotes: 4

Related Questions