correctly close struct file * on module_exit

user_10563808 0
Viewed 317

I'm learning how to write linux driver, but I have some confusion about how to use kernel API correctly and elegantly. I try to write a simple misc driver, it creates a device node on /dev/hello. In userspace, readers can read on the device and block until writers write some data on the device, all readers get the data written. If new data comes before the reader reads the old data, the reader will lose the old data.

To implement it, I create a struct hello_file_data for every opened file and put them on a global link-list, the data field indicates there are some data that can be read.

static LIST_HEAD(hello_opened_file_list);

struct hello_file_data {
    struct list_head    entry;
    struct file *       owner_file;
    int                 data;
};

In the read function, I use wait_event_interruptible to block this thread and wait for the file's struct hello_file_data's data field become 1.

static ssize_t hello_read(struct file * file, char __user * data, size_t n, loff_t * offset_p)
{
    int res;
    struct hello_file_data * fdat = file->private_data;

    res = wait_event_interruptible(hello_wait_data, hello_dead || fdat->data);
    if (res) {
        return res;
    }
    if (hello_dead) {
        return -ENODEV;
    }
    n = min(hello_bufsize, n);
    if (copy_to_user(data, hello_buffer, n)) {
        return -EFAULT;
    }
    fdat->data = 0;
    return n;
}

In the write function, I iterate the global linked-list to set every file's struct hello_file_data's data field to 1 then notify all the reader threads to wakeup.

static ssize_t hello_write(struct file * file, const char __user * data, size_t n, loff_t * offset_p)
{
    struct hello_file_data * fdat = file->private_data;

    if (!n) {
        return 0;
    }
    n = min(sizeof(hello_buffer), n);
    if (copy_from_user(hello_buffer, data, n)) {
        return -EFAULT;
    }
    hello_bufsize = n;
    spin_lock(&hello_list_lock);
    list_for_each_entry(fdat, &hello_opened_file_list, entry) {
        fdat->data = 1;
    }
    spin_unlock(&hello_list_lock);
    wake_up_interruptible(&hello_wait_data);
    return n;
}

I have three puzzles on the code

  1. When module_exit is called, I must wait until all struct file * are closed correctly, is it appropriate to use wait_event?
  2. When this kernel module will be removed by rmmod, I have no way to close(detach?) struct file *, so will the rmmod command block until these files are closed by program; is there a better way to handle it?
  3. When iterating all struct file *, is there a way to use a kernel API rather than managing my own linked-list?

hello.c

#include <linux/init.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/fs.h>
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/miscdevice.h>
#include <linux/uaccess.h>

static LIST_HEAD(hello_opened_file_list);
static DEFINE_SPINLOCK(hello_list_lock);
static DECLARE_WAIT_QUEUE_HEAD(hello_wait_data);
static DECLARE_WAIT_QUEUE_HEAD(hello_wait_all_file_close);

static char     hello_buffer[1024];
static size_t   hello_bufsize = 0;
static int      hello_dead = 0;

struct hello_file_data {
    struct list_head    entry;
    struct file *       owner_file;
    int                 data;
};

static int hello_open(struct inode * inode, struct file * file)
{
    struct hello_file_data * fdat;

    fdat = kzalloc(sizeof(struct hello_file_data), GFP_KERNEL);
    if (!fdat) {
        return -ENOMEM;
    }
    fdat->owner_file = file;
    fdat->data = 0;
    file->private_data = fdat;
    spin_lock(&hello_list_lock);
    list_add(&fdat->entry, &hello_opened_file_list);
    spin_unlock(&hello_list_lock);
    return 0;
}

static int hello_release(struct inode * inode, struct file * file)
{
    struct hello_file_data * fdat = file->private_data;
    int notify_module_exit = 0;

    spin_lock(&hello_list_lock);
    list_del(&fdat->entry);
    if (hello_dead && list_empty(&hello_opened_file_list)) {
        notify_module_exit = 1;
    }
    spin_unlock(&hello_list_lock);
    file->private_data = NULL;
    kfree(fdat);

    if (notify_module_exit) {
        wake_up(&hello_wait_all_file_close);
    }
    return 0;
}

static ssize_t hello_read(struct file * file, char __user * data, size_t n, loff_t * offset_p)
{
    int res;
    struct hello_file_data * fdat = file->private_data;

    res = wait_event_interruptible(hello_wait_data, hello_dead || fdat->data);
    if (res) {
        return res;
    }
    if (hello_dead) {
        return -ENODEV;
    }
    n = min(hello_bufsize, n);
    if (copy_to_user(data, hello_buffer, n)) {
        return -EFAULT;
    }
    fdat->data = 0;
    return n;
}

static ssize_t hello_write(struct file * file, const char __user * data, size_t n, loff_t * offset_p)
{
    struct hello_file_data * fdat = file->private_data;

    if (!n) {
        return 0;
    }
    n = min(sizeof(hello_buffer), n);
    if (copy_from_user(hello_buffer, data, n)) {
        return -EFAULT;
    }
    hello_bufsize = n;
    spin_lock(&hello_list_lock);
    list_for_each_entry(fdat, &hello_opened_file_list, entry) {
        fdat->data = 1;
    }
    spin_unlock(&hello_list_lock);
    wake_up_interruptible(&hello_wait_data);
    return n;
}

static struct file_operations hello_fops = {
    .open       = hello_open,
    .read       = hello_read,
    .write      = hello_write,
    .release    = hello_release,
};

static struct miscdevice hellodev = {
    .minor = MISC_DYNAMIC_MINOR,
    .name = "hello",
    .fops = &hello_fops,
};

static int hello_module_init(void)
{
    return misc_register(&hellodev);
}

static void hello_module_exit(void)
{
    misc_deregister(&hellodev);
    hello_dead = 1;
    wake_up_interruptible(&hello_wait_data);
    wait_event(hello_wait_all_file_close, ({
        int empty;
        spin_lock(&hello_list_lock);
        empty = list_empty(&hello_opened_file_list);
        spin_unlock(&hello_list_lock);
        empty;
    }));
}

module_init(hello_module_init);
module_exit(hello_module_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("xfan");
MODULE_DESCRIPTION("This is test driver");
linux-device-driver linux-kernel linux c
1 Answers

That's not the right way to do it. In fact, the right way is far simpler for you. The kernel build framework in conjunction with the kernel run-time loader will build a struct module for your module (called "THIS_MODULE"). You need to place a pointer to it in the .owner slot of the file_operations structure. This is easy to do:

static struct file_operations hello_fops = {
    .owner      = THIS_MODULE,        // <<<<<<========
    .open       = hello_open,
    .read       = hello_read,
    .write      = hello_write,
    .release    = hello_release,
};

The way this works is that the kernel tracks all the open files that belong to your module (through that owner member). That is, it increments a reference count associated with your module whenever a device instance is opened. When the device instance is closed, your hello_release is called, and then the module reference count is decremented. The kernel won't allow your module to be unloaded while it still owns open files, so you would need to track down and kill any user processes that hold file references before unloading your module. That's the only way to do this reliably (and it sounds like that's what you want anyway).

Unfortunately, a lot of the kernel module/driver samples were created back in the old days when the module had to do its own reference counting with try_module_get and module_put, so a lot of samples don't explain how this works. The get/put mechanism had races though: the module really can't reliably reference-count itself.

If you do this, then you don't need to worry about it: when your module_exit is called, you can be assured that there are no open device instances that belong to your module.

user_10764790
Related