Correct way to initialize semaphore in linux driver

Question

I wrote this linux char driver just to control open call,

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
#include <linux/semaphore.h>
#include <linux/device.h>
#include <linux/cdev.h>

MODULE_LICENSE("GPL");
#define CLASS_NAME "myclass"
#define MINOR_NUM 0
#define MINOR_CNT 1
static struct class *myclass=NULL;
static struct device *mydevice=NULL;
static dev_t mycdevt;
static struct cdev *mycdev;
static struct semaphore *sem; 

static int myopen(struct inode *inod, struct file *fp)
{
    down(sem);
    printk(KERN_INFO "critical section\n");
    return 0;
}

static int myclose(struct inode *inod, struct file *fp )
{
    up(sem);
    printk(KERN_INFO "critical section freed\n");
    return 0;
}

static ssize_t myread(struct file *fp, char *buf, size_t len, loff_t *off)
{
    return 0;
}

static ssize_t mywrite(struct file *fp, char *buf, size_t len, loff_t *off)
{
    return 0;
}

static struct file_operations fops = 
{
    .open = myopen,
    .release = myclose,
    .read = myread,
    .write = mywrite,
};

static int __init myinit(void)
{
    int ret;

    ret = alloc_chrdev_region ( &mycdevt, MINOR_NUM, MINOR_CNT, "mycdevt");
    if(ret<0)
    {
        printk(KERN_INFO "chardev can't be allocated\n");
    //  goto label;//todo
    }

    mycdev = cdev_alloc();//instead of cdev_alloc, we can use cdev_init(&mycdev, &fops);
    if(mycdev == NULL)
    {
        printk(KERN_INFO"cdev_alloc failed\n");
    //  goto label;//todo
    }
    mycdev->ops = &fops;

    ret = cdev_add(mycdev, mycdevt, 1);
    if(ret < 0)
    {
        printk(KERN_INFO"cdev_add failed\n");
    //  goto label;//todo
    }

    myclass = class_create(THIS_MODULE, CLASS_NAME);
    if(myclass == NULL)
    {
        printk(KERN_INFO"class create failed\n");
    //  goto label;//todo
    }


    mydevice = device_create(myclass, NULL, mycdevt, NULL, "mydevice");
    if(mydevice == NULL)
    {
        printk(KERN_INFO"device create failed\n");
    //  goto label;//todo
    }

    sema_init(sem, 1);//here is the problem
    printk(KERN_INFO"myinit done\n");
    return 0;
}

static void __exit myexit(void)
{
    device_destroy(myclass, mycdevt);
    class_unregister(myclass);
    class_destroy(myclass);
    cdev_del(mycdev);
    unregister_chrdev(MAJOR(mycdevt), "mycdevt");
    printk(KERN_INFO "exited\n");
}   

module_init(myinit);
module_exit(myexit);

I am following ldd3 book, and tried to use simple APIs to use semahore for an app. What happened is my kernel crashed when it called sema_init function. I read that semaphore is used in mutex mode http://www.makelinux.net/ldd3/chp-5-sect-3

I also read that semaphore is not a mutex as there is no ownership. I have yet to explore that 'ownership' thing, but for now I am not able to make a simple semaphore.

What wrong I am doing here?

It should be noted that using a struct semaphore as a mutex is a bit out-of-date. Unless you need to support old kernels before 2.6.16, you should probably be using the newer struct mutex. — Ian Abbott
sometime people say, mutex is a lot different than semaphore and books say that its just a binary semaphore. What to follow — acidlategamer
There are different types of mutexes, including reentrant mutexes where the mutex can be recursively locked by the same owner. The struct mutex in the kernel is just a simple, non-reentrant mutex. — Ian Abbott
Just read linux professional architecture book. It mentioned two types of mutex, one mutex is used as a binary semaphore and called classical mutetx, the other that you are telling about is used to counter priority inversion problem. — acidlategamer
So since the re-entrant mutex needs to record the owner, the priority of the current owner can be bumped up by the OS while a higher priority task is waiting for the mutex? Implementing re-entrancy and overcoming priority inversion both require the current owner of the mutex to be recorded, but they are not the same thing. — Ian Abbott

Andrew Henle Andrew Henle · Accepted Answer · 2017-03-25T20:36:50

There's no actual semaphore.

This line:

static struct semaphore *sem;

creates a pointer to a semaphore, not a semaphore itself. It's almost certain initialized to NULL.

You probably want something like

static struct semaphore sem;

and therefore

sema_init( &sem, 1);

along with

up( &sem );

and

down( &sem );

Note that these calls are taking the address of an actual semaphore.

Correct way to initialize semaphore in linux driver

1 Answers