lkml.org 
[lkml]   [2006]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 2/9] Add new generic HW RNG core
Michael Buesch <mb@bu3sch.de> wrote:
>
> ...
>
> +static inline
> +int hwrng_init(struct hwrng *rng)
> +static inline
> +void hwrng_cleanup(struct hwrng *rng)
> +static inline
> +int hwrng_data_present(struct hwrng *rng)
> +static inline
> +int hwrng_data_read(struct hwrng *rng, u32 *data)

Lose the newlines, please.

> +static int rng_dev_open(struct inode *inode, struct file *filp)

Like that.

> +static ssize_t rng_dev_read(struct file *filp, char __user *buf,
> + size_t size, loff_t *offp)
> +{
> + int have_data;
> + u32 data;
> + ssize_t ret = 0;
> + int i, err = 0;
> +
> + while (size) {
> + err = -ERESTARTSYS;
> + if (mutex_lock_interruptible(&rng_mutex))
> + goto out;
> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + err = -ENODEV;
> + goto out;
> + }
> + have_data = 0;
> + if (hwrng_data_present(current_rng))
> + have_data = hwrng_data_read(current_rng, &data);
> + mutex_unlock(&rng_mutex);
> +
> + err = -EFAULT;
> + while (have_data && size) {
> + if (put_user((u8)data, buf++))
> + goto out;
> + size--;
> + ret++;
> + have_data--;
> + data >>= 8;
> + }
> +
> + err = -EAGAIN;
> + if (filp->f_flags & O_NONBLOCK)
> + goto out;
> +
> + err = -ERESTARTSYS;
> + if (need_resched()) {
> + if (schedule_timeout_interruptible(1))
> + goto out;
> + } else {
> + if (mutex_lock_interruptible(&rng_mutex))
> + goto out;
> + if (!current_rng) {
> + mutex_unlock(&rng_mutex);
> + err = -ENODEV;
> + goto out;
> + }
> + for (i = 0; i < 20; i++) {
> + if (hwrng_data_present(current_rng))
> + break;
> + udelay(10);
> + }
> + mutex_unlock(&rng_mutex);
> + }
> + if (signal_pending(current))
> + goto out;
> + }
> +out:
> + return ret ? : err;
> +}

What's going on with the need_resched() tricks in there? (Unobvious, needs
a comment). From my reading, it'll cause a caller to this function to hang
for arbitrary periods of time if something if causing heavy scheduling
pressure.

What's the polling of hwrng_data_present() doing in here? (Unobvious,
needs a comment).

> +static ssize_t hwrng_attr_current_store(struct class_device *class,
> + const char *buf, size_t len)
> +{
> + int err;
> + struct hwrng *rng;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;

Are the sysfs permissions not adequate?

> +MODULE_AUTHOR("The Linux Kernel team");

Mutter. Might as well remove this.

A MAINTAINERS record would be nice.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-05-16 00:02    [W:0.060 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site