endlessly looping when reading from character device

Question

For a homework assignment, I have written a character device driver. It seems to work OK. I can read and write it. The problem is that when I read the device, it endlessly loops, printing out the contents of the message buffer over and over.

This seems like it should be fairly straight forward. Just use copy_to_user(), but it's proven to be very problematic.

Anyway, here is the code. I think the problem is in the gdev_read() function. The printk's are there to serve as debugging as well as talking points, since I have to present the project in class.

/*
 * Implement a generic character pseudo-device driver
 */

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fs.h>
#include <linux/cdev.h>
#include <linux/types.h>
#include <linux/vmalloc.h>
#include <asm/uaccess.h>

/* you need these, or the kernel will be tainted */
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("A simple sample character device driver");

/*
 * function prototypes
 */
int init_module(void);
void cleanup_module(void);
static ssize_t gdev_read(struct file *, char *, size_t, loff_t *);
static ssize_t gdev_write(struct file *, const char *, size_t, loff_t *);
static int gdev_open(struct inode *, struct file *);
static int gdev_release(struct inode *, struct file *);

/* macros */
#define TRUE 1
#define FALSE 0
#define MAX_MSG_LEN 64

/*
 * global variables
 */
static dev_t dev_num;   /* device number, for new device */
static char *mesg;  /* buffer for message */


/* file operations structure, so my device knows how to act */
static struct file_operations fops = {
    .owner =    THIS_MODULE,
    .read =     gdev_read,
    .write =    gdev_write,
    .open =     gdev_open,
    .release =  gdev_release,
};

/* character device struct.  Declaired here, but initialized elsewhere */
struct cdev *gdev;

int init_module(void)
{   
    int err;
    printk(KERN_ALERT "in init_module\n");

    if(alloc_chrdev_region(&dev_num, 0, 1, "/dev/gdev")){
        printk(KERN_INFO "Could not allocate device numbers\n");
        printk(KERN_INFO "Module gdev not loaded\n");
        return -1;
    }

    /* now I need to make the device and register it */
    gdev = cdev_alloc();
    gdev->owner = THIS_MODULE;
    gdev->ops = &fops;
    err = cdev_add(gdev, dev_num, 1);
    if(err){
        printk(KERN_NOTICE "Error %d adding gdev", err);
        return err;
    }

    mesg = (char *)vmalloc(MAX_MSG_LEN);

    printk(KERN_INFO "Module gdev successfully loaded.\n");
    printk(KERN_INFO "gdev Major Number: %d\n", MAJOR(dev_num));

    return 0;
}


void cleanup_module(void)
{
    printk(KERN_ALERT "in cleanup_module\n");
    unregister_chrdev_region(dev_num, 3);
    vfree(mesg);
    cdev_del( gdev );
    printk(KERN_INFO "Module gdev unregistered\n");
}

static ssize_t gdev_read(struct file *filp, char *page, 
            size_t len, loff_t *offset)
{
    ssize_t bytes = len < MAX_MSG_LEN ? len : MAX_MSG_LEN;
    printk(KERN_ALERT "in gdev_read\n");
    if(copy_to_user(page, mesg, bytes)){
        return -EFAULT;
    }
    return bytes;
}

static ssize_t gdev_write(struct file *filp, const char *page, 
            size_t len, loff_t *offset)
{
    ssize_t bytes = len < MAX_MSG_LEN ? len : MAX_MSG_LEN; 
    printk(KERN_ALERT "in gdev_write\n");
    if(copy_from_user(mesg, page, bytes)){
        return -EFAULT;
    }

    return bytes;
}

static int gdev_open(struct inode *inode, struct file *filp)
{
    printk(KERN_ALERT "in gdev_open\n");
    return 0;
}

static int gdev_release(struct inode *inode, struct file *filp)
{
    printk(KERN_ALERT "in gdev_release\n");
    /* doesn't do anything because it doesn't need too */
    return 0;
}

Answer 1

You could use 'simple_read_from_buffer' function from 'linux/fs.h':

static ssize_t gdev_read(struct file *filep, char __user *buff, size_t count, loff_t *offp)
{
    return simple_read_from_buffer(buff, count, offp, my_buffer, buffer_len);
}

'my_buffer' and 'buffer_len' are defined in your module.

Answer 2

If zero is not returned from read() (in your case gdev_read()), the read function will be called again. To stop this you use the loff_t *offset parameter. Increment it by how many bytes you have read using (*offset) += bytes; after copy_to_user(). Next time read() is called, offset will be what you have incremented it to. Now just check how many bytes you have previously sent, and only send what you still have remaining. Your function should look like this:

static ssize_t gdev_read(struct file *filp, char *page, 
            size_t len, loff_t *offset)
{
    ssize_t bytes = len < (MAX_MSG_LEN-(*offset)) ? len : (MAX_MSG_LEN-(*offset));
    printk(KERN_ALERT "in gdev_read\n");
    if(copy_to_user(page, mesg, bytes)){
        return -EFAULT;
    }
    (*offset) += bytes;
    return bytes;
}