[RFC][PATCH 1/1] support for user-space buffers in kfifo

[RFC][PATCH 1/1] support for user-space buffers in kfifo

[Nelson Castillo]

[-- Attachment #1: Type: text/plain, Size: 7876 bytes --]

Hi.

I just added support for user space buffers in kfifo. I found useful
__kfifo_get_user to copy data to a user buffer in a read call. I didn't
like the idea of having an extra buffer.

* Is it ok to add this support?
* Am I missing something?

I expect to fill the fifo using __kfifo_put in interrupt context.
If I understand correctly, I won't need extra locking if I have a
single interrupt handler filling this buffer and all the readers use a
semaphore.

This is how I'm using it now:

static ssize_t device_read(struct file *filep, char __user *buffer,
			   size_t length, loff_t * offset)
{
	struct sarpic_dev *dev = filep->private_data;
	int ret_val;

	if (down_interruptible (&dev->sem))
		return -ERESTARTSYS;

	while (__kfifo_len(dev->fifo) == 0) {
		up(&dev->sem);

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

		if (wait_event_interruptible(dev->readq, (__kfifo_len(dev->fifo) != 0)))
			return -ERESTARTSYS;

		if (down_interruptible(&dev->sem))
			return -ERESTARTSYS;
	}

	ret_val = __kfifo_get_user(dev->fifo, buffer, length);

	up(&dev->sem);

	return ret_val;
}

static ssize_t
device_write(struct file *filep,
             const char __user *buffer, size_t length, loff_t *offset)
{
	struct sarpic_dev *dev = filep->private_data;
	int ret_val;

  if (down_interruptible (&dev->sem))
    return -ERESTARTSYS;

  ret_val = __kfifo_put_user(dev->fifo, buffer, length);

  up(&dev->sem);

  wake_up_interruptible(&dev->readq);

  return ret_val;
}

Regards,
Nelson.-


Signed-off-by: Nelson Castillo <nelson@emqbit.com>
---
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 404f446..4568285 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -41,8 +41,13 @@ extern struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask,
 extern void kfifo_free(struct kfifo *fifo);
 extern unsigned int __kfifo_put(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_put_user(struct kfifo *fifo,
+			    const unsigned char __user *buffer,
+			    unsigned int len);
 extern unsigned int __kfifo_get(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_get_user(struct kfifo *fifo,  unsigned char __user *buffer,
+			    unsigned int len);
 
 /**
  * __kfifo_reset - removes the entire FIFO contents, no locking version
@@ -94,6 +99,32 @@ static inline unsigned int kfifo_put(struct kfifo *fifo,
 }
 
 /**
+ * kfifo_put_user - puts some user data into the FIFO
+ * @fifo: the fifo to be used.
+ * @buffer: the user-space data to be added.
+ * @len: the length of the data to be added.
+ *
+ * This function copies at most @len bytes from the @buffer into
+ * the FIFO depending on the free space, and returns the number of
+ * bytes copied.
+ */
+static inline int kfifo_put_user(struct kfifo *fifo,
+				 const unsigned char __user *buffer,
+			         unsigned int len)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(fifo->lock, flags);
+
+	ret = __kfifo_put_user(fifo, buffer, len);
+
+	spin_unlock_irqrestore(fifo->lock, flags);
+
+	return ret;
+}
+
+/**
  * kfifo_get - gets some data from the FIFO
  * @fifo: the fifo to be used.
  * @buffer: where the data must be copied.
@@ -125,6 +156,34 @@ static inline unsigned int kfifo_get(struct kfifo *fifo,
 }
 
 /**
+ * kfifo_get_user - gets some data from the FIFO an user buffer
+ * @fifo: the fifo to be used.
+ * @buffer: user buffer where the data must be copied
+ * @len: the size of the destination buffer.
+ *
+ * This function copies at most @len bytes from the FIFO into the
+ * @buffer and returns the number of copied bytes.
+ */
+
+static inline int kfifo_get_user(struct kfifo *fifo,
+				     unsigned char __user *buffer, unsigned int len)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(fifo->lock, flags);
+
+	ret = __kfifo_get_user(fifo, buffer, len);
+
+	if (fifo->in == fifo->out)
+		fifo->in = fifo->out = 0;
+
+	spin_unlock_irqrestore(fifo->lock, flags);
+
+	return ret;
+}
+
+/**
  * __kfifo_len - returns the number of bytes available in the FIFO, no locking version
  * @fifo: the fifo to be used.
  */
diff --git a/kernel/kfifo.c b/kernel/kfifo.c
index cee4191..e95ab37 100644
--- a/kernel/kfifo.c
+++ b/kernel/kfifo.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <asm/uaccess.h>
 #include <linux/kfifo.h>
 
 /**
@@ -150,6 +151,59 @@ unsigned int __kfifo_put(struct kfifo *fifo,
 EXPORT_SYMBOL(__kfifo_put);
 
 /**
+ * __kfifo_put_user - puts some data into the FIFO, from userspace
+ *		       no locking version
+ * @fifo: the fifo to be used.
+ * @buffer: the user space data to be added.
+ * @len: the length of the data to be added.
+ *
+ * This function copies at most @len bytes from the @buffer into
+ * the FIFO depending on the free space, and returns the number of
+ * bytes copied.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
+	        unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->size - fifo->in + fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->out index -before- we
+	 * start putting bytes into the kfifo.
+	 */
+
+	smp_mb();
+
+	/* first put the data starting from fifo->in to buffer end */
+	l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
+
+	if(copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
+			  buffer, l))
+		return  -EFAULT;
+
+
+	/* then put the rest (if any) at the beginning of the buffer */
+	if (copy_from_user(fifo->buffer, buffer + l, len - l))
+		return  -EFAULT;
+
+	/*
+	 * Ensure that we add the bytes to the kfifo -before-
+	 * we update the fifo->in index.
+	 */
+
+	smp_wmb();
+
+	fifo->in += len;
+
+	return (int)len;
+}
+EXPORT_SYMBOL(__kfifo_put_user);
+
+/**
  * __kfifo_get - gets some data from the FIFO, no locking version
  * @fifo: the fifo to be used.
  * @buffer: where the data must be copied.
@@ -193,4 +247,58 @@ unsigned int __kfifo_get(struct kfifo *fifo,
 
 	return len;
 }
+
 EXPORT_SYMBOL(__kfifo_get);
+
+/**
+ * __kfifo_get_user - gets some data from the FIFO to userspace
+ *		      no blocking version
+ * @fifo: the fifo to be used.
+ * @buffer: user space buffer where the data must be copied.
+ * @len: the size of the destination buffer.
+ *
+ * This function copies at most @len bytes from the FIFO into the
+ * @buffer and returns the number of copied bytes.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+
+int __kfifo_get_user(struct kfifo *fifo,
+			 unsigned char __user *buffer, unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->in - fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->in index -before- we
+	 * start removing bytes from the kfifo.
+	 */
+
+	smp_rmb();
+
+	/* first get the data from fifo->out until the end of the buffer */
+	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
+
+	if (copy_to_user(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l))
+		return -EFAULT;
+
+	/* then get the rest (if any) from the beginning of the buffer */
+	if (copy_to_user(buffer + l, fifo->buffer, len - l))
+		return -EFAULT;
+
+	/*
+	 * Ensure that we remove the bytes from the kfifo -before-
+	 * we update the fifo->out index.
+	 */
+
+	smp_mb();
+
+	fifo->out += len;
+
+	return (int)len;
+}
+
+EXPORT_SYMBOL(__kfifo_get_user);
+

-- 

http://arhuaco.org
http://emQbit.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Stelian Pop]

Le lundi 11 juin 2007 à 16:26 -0500, Nelson Castillo a écrit :
> Hi.
> 
> I just added support for user space buffers in kfifo. I found useful
> __kfifo_get_user to copy data to a user buffer in a read call. I didn't
> like the idea of having an extra buffer.
> 
> * Is it ok to add this support?

I suppose it is, however:

> +	spin_lock_irqsave(fifo->lock, flags);
> +
> +	ret = __kfifo_put_user(fifo, buffer, len);
> +
> +	spin_unlock_irqrestore(fifo->lock, flags);
[...]

> +int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
> +	        unsigned int len)
[...]

> +	if(copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
> +			  buffer, l))
> +		return  -EFAULT;
[...]

accessing userspace memory with a spinlock taken (moreover an irqsave()
one) is bad bad bad.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Nelson Castillo]

On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> Le lundi 11 juin 2007 à 16:26 -0500, Nelson Castillo a écrit :
> > Hi.
> >
> > I just added support for user space buffers in kfifo. I found useful
> > __kfifo_get_user to copy data to a user buffer in a read call. I didn't
> > like the idea of having an extra buffer.
> >
> > * Is it ok to add this support?
>
> I suppose it is, however:
>
> > +     spin_lock_irqsave(fifo->lock, flags);
> > +
> > +     ret = __kfifo_put_user(fifo, buffer, len);
> > +
> > +     spin_unlock_irqrestore(fifo->lock, flags);
> [...]
>
> > +int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
> > +             unsigned int len)
> [...]
>
> > +     if(copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
> > +                       buffer, l))
> > +             return  -EFAULT;
> [...]
>
> accessing userspace memory with a spinlock taken (moreover an irqsave()
> one) is bad bad bad.

Yes.

Perhaps then only the functions __kfifo_put_user and __kfifo_get_user should
be allowed. Those are the ones I'm using now , anyway.
I added the others for completeness but adding them is probably a mistake.

Regards,
Nelson.-

-- 
http://arhuaco.org
http://emQbit.com

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Nelson Castillo]

[-- Attachment #1: Type: text/plain, Size: 4928 bytes --]

On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
(cut)
> accessing userspace memory with a spinlock taken (moreover an
> irqsave()
> one) is bad bad bad.

Hi Stelian.

I'm sending the new patch without kfifo_put_user and kfifo_get_user.

Regards,
Nelson.-

Signed-off-by: Nelson Castillo <nelson@emqbit.com>
---
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 404f446..5c4c416 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -41,8 +41,13 @@ extern struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask,
 extern void kfifo_free(struct kfifo *fifo);
 extern unsigned int __kfifo_put(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_put_user(struct kfifo *fifo,
+			    const unsigned char __user *buffer,
+			    unsigned int len);
 extern unsigned int __kfifo_get(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_get_user(struct kfifo *fifo,  unsigned char __user *buffer,
+			    unsigned int len);
 
 /**
  * __kfifo_reset - removes the entire FIFO contents, no locking version
diff --git a/kernel/kfifo.c b/kernel/kfifo.c
index cee4191..41438ec 100644
--- a/kernel/kfifo.c
+++ b/kernel/kfifo.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <asm/uaccess.h>
 #include <linux/kfifo.h>
 
 /**
@@ -150,6 +151,61 @@ unsigned int __kfifo_put(struct kfifo *fifo,
 EXPORT_SYMBOL(__kfifo_put);
 
 /**
+ * __kfifo_put_user - puts some data into the FIFO, from userspace
+ *		      We don't have a locking version because copy_from_user
+ *		      might sleep and you must not sleep holding a spinlock.
+ *
+ * @fifo: the fifo to be used.
+ * @buffer: the user space data to be added.
+ * @len: the length of the data to be added.
+ *
+ * This function copies at most @len bytes from the @buffer into
+ * the FIFO depending on the free space, and returns the number of
+ * bytes copied.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
+	        unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->size - fifo->in + fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->out index -before- we
+	 * start putting bytes into the kfifo.
+	 */
+
+	smp_mb();
+
+	/* first put the data starting from fifo->in to buffer end */
+	l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
+
+	if(copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
+			  buffer, l))
+		return  -EFAULT;
+
+
+	/* then put the rest (if any) at the beginning of the buffer */
+	if (copy_from_user(fifo->buffer, buffer + l, len - l))
+		return  -EFAULT;
+
+	/*
+	 * Ensure that we add the bytes to the kfifo -before-
+	 * we update the fifo->in index.
+	 */
+
+	smp_wmb();
+
+	fifo->in += len;
+
+	return (int)len;
+}
+EXPORT_SYMBOL(__kfifo_put_user);
+
+/**
  * __kfifo_get - gets some data from the FIFO, no locking version
  * @fifo: the fifo to be used.
  * @buffer: where the data must be copied.
@@ -194,3 +250,56 @@ unsigned int __kfifo_get(struct kfifo *fifo,
 	return len;
 }
 EXPORT_SYMBOL(__kfifo_get);
+
+/**
+ * __kfifo_get_user - gets some data from the FIFO to userspace
+ *		      We don't have a locking version because copy_to_user
+ *		      might sleep and you must not sleep holding a spinlock.
+ * @fifo: the fifo to be used.
+ * @buffer: user space buffer where the data must be copied.
+ * @len: the size of the destination buffer.
+ *
+ * This function copies at most @len bytes from the FIFO into the
+ * @buffer and returns the number of copied bytes.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+
+int __kfifo_get_user(struct kfifo *fifo,
+			 unsigned char __user *buffer, unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->in - fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->in index -before- we
+	 * start removing bytes from the kfifo.
+	 */
+
+	smp_rmb();
+
+	/* first get the data from fifo->out until the end of the buffer */
+	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
+
+	if (copy_to_user(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l))
+		return -EFAULT;
+
+	/* then get the rest (if any) from the beginning of the buffer */
+	if (copy_to_user(buffer + l, fifo->buffer, len - l))
+		return -EFAULT;
+
+	/*
+	 * Ensure that we remove the bytes from the kfifo -before-
+	 * we update the fifo->out index.
+	 */
+
+	smp_mb();
+
+	fifo->out += len;
+
+	return (int)len;
+}
+EXPORT_SYMBOL(__kfifo_get_user);
+

-- 

http://arhuaco.org
http://emQbit.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Stelian Pop]

Le mardi 12 juin 2007 à 11:24 -0500, Nelson Castillo a écrit :
> On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> (cut)
> > accessing userspace memory with a spinlock taken (moreover an
> > irqsave()
> > one) is bad bad bad.
> 
> Hi Stelian.
> 
> I'm sending the new patch without kfifo_put_user and kfifo_get_user.

Ok, I have a few formatting/documentation comments:

> + * This function copies at most @len bytes from the @buffer into
> + * the FIFO depending on the free space, and returns the number of
> + * bytes copied.

... or -EFAULT if copy_from_user fails

> + *
> + * Note that with only one concurrent reader and one concurrent
> + * writer, you don't need extra locking to use these functions.
> + */
> +int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
> +	        unsigned int len)

spacing ?

> +	if(copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
> +			  buffer, l))

leave a space after the if please

> + * This function copies at most @len bytes from the FIFO into the
> + * @buffer and returns the number of copied bytes.

or -EFAULT

> + *
> + * Note that with only one concurrent reader and one concurrent
> + * writer, you don't need extra locking to use these functions.
> + */
> +
> +int __kfifo_get_user(struct kfifo *fifo,
> +			 unsigned char __user *buffer, unsigned int len)

spacing.

Other than that I'm ok with this patch.

Thanks,

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Nelson Castillo]

[-- Attachment #1: Type: text/plain, Size: 5295 bytes --]

On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> Le mardi 12 juin 2007 à 11:24 -0500, Nelson Castillo a écrit :
> > On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> > (cut)
> > > accessing userspace memory with a spinlock taken (moreover an
> > > irqsave()
> > > one) is bad bad bad.
> >
> > Hi Stelian.
> >
> > I'm sending the new patch without kfifo_put_user and kfifo_get_user.
> 
> Ok, I have a few formatting/documentation comments:

Applied, thanks.

> Other than that I'm ok with this patch.

Submitting.

Regards, Nelson.-

Signed-off-by: Nelson Castillo <nelson@emqbit.com>
---
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 404f446..5c4c416 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -41,8 +41,13 @@ extern struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask,
 extern void kfifo_free(struct kfifo *fifo);
 extern unsigned int __kfifo_put(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_put_user(struct kfifo *fifo,
+			    const unsigned char __user *buffer,
+			    unsigned int len);
 extern unsigned int __kfifo_get(struct kfifo *fifo,
 				unsigned char *buffer, unsigned int len);
+extern int __kfifo_get_user(struct kfifo *fifo,  unsigned char __user *buffer,
+			    unsigned int len);
 
 /**
  * __kfifo_reset - removes the entire FIFO contents, no locking version
diff --git a/kernel/kfifo.c b/kernel/kfifo.c
index cee4191..b11024e 100644
--- a/kernel/kfifo.c
+++ b/kernel/kfifo.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <asm/uaccess.h>
 #include <linux/kfifo.h>
 
 /**
@@ -150,6 +151,61 @@ unsigned int __kfifo_put(struct kfifo *fifo,
 EXPORT_SYMBOL(__kfifo_put);
 
 /**
+ * __kfifo_put_user - puts some data into the FIFO, from userspace
+ *		      We don't have a locking version because copy_from_user
+ *		      might sleep and you must not sleep holding a spinlock.
+ *
+ * @fifo: the fifo to be used.
+ * @buffer: the user space data to be added.
+ * @len: the length of the data to be added.
+ *
+ * This function copies at most @len bytes from the @buffer into
+ * the FIFO depending on the free space, and returns the number of
+ * bytes copied or -EFAULT if copy_from_user fails.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+int __kfifo_put_user(struct kfifo *fifo, const unsigned char __user *buffer,
+		     unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->size - fifo->in + fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->out index -before- we
+	 * start putting bytes into the kfifo.
+	 */
+
+	smp_mb();
+
+	/* first put the data starting from fifo->in to buffer end */
+	l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
+
+	if (copy_from_user(fifo->buffer + (fifo->in & (fifo->size - 1)),
+			   buffer, l))
+		return  -EFAULT;
+
+
+	/* then put the rest (if any) at the beginning of the buffer */
+	if (copy_from_user(fifo->buffer, buffer + l, len - l))
+		return  -EFAULT;
+
+	/*
+	 * Ensure that we add the bytes to the kfifo -before-
+	 * we update the fifo->in index.
+	 */
+
+	smp_wmb();
+
+	fifo->in += len;
+
+	return (int)len;
+}
+EXPORT_SYMBOL(__kfifo_put_user);
+
+/**
  * __kfifo_get - gets some data from the FIFO, no locking version
  * @fifo: the fifo to be used.
  * @buffer: where the data must be copied.
@@ -194,3 +250,57 @@ unsigned int __kfifo_get(struct kfifo *fifo,
 	return len;
 }
 EXPORT_SYMBOL(__kfifo_get);
+
+/**
+ * __kfifo_get_user - gets some data from the FIFO to userspace
+ *		      We don't have a locking version because copy_to_user
+ *		      might sleep and you must not sleep holding a spinlock.
+ * @fifo: the fifo to be used.
+ * @buffer: user space buffer where the data must be copied.
+ * @len: the size of the destination buffer.
+ *
+ * This function copies at most @len bytes from the FIFO into the
+ * @buffer and returns the number of bytes copied or -EFAULT if
+ * copy_to_user fails.
+ *
+ * Note that with only one concurrent reader and one concurrent
+ * writer, you don't need extra locking to use these functions.
+ */
+
+int __kfifo_get_user(struct kfifo *fifo,
+		     unsigned char __user *buffer, unsigned int len)
+{
+	unsigned int l;
+
+	len = min(len, fifo->in - fifo->out);
+
+	/*
+	 * Ensure that we sample the fifo->in index -before- we
+	 * start removing bytes from the kfifo.
+	 */
+
+	smp_rmb();
+
+	/* first get the data from fifo->out until the end of the buffer */
+	l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
+
+	if (copy_to_user(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l))
+		return -EFAULT;
+
+	/* then get the rest (if any) from the beginning of the buffer */
+	if (copy_to_user(buffer + l, fifo->buffer, len - l))
+		return -EFAULT;
+
+	/*
+	 * Ensure that we remove the bytes from the kfifo -before-
+	 * we update the fifo->out index.
+	 */
+
+	smp_mb();
+
+	fifo->out += len;
+
+	return (int)len;
+}
+EXPORT_SYMBOL(__kfifo_get_user);
+

-- 

http://arhuaco.org
http://emQbit.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC][PATCH 1/1] support for user-space buffers in kfifo

[Stelian Pop]

Le mardi 12 juin 2007 à 21:21 -0500, Nelson Castillo a écrit :
> On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> > Le mardi 12 juin 2007 à 11:24 -0500, Nelson Castillo a écrit :
> > > On 6/12/07, Stelian Pop <stelian@popies.net> wrote:
> > > (cut)
> > > > accessing userspace memory with a spinlock taken (moreover an
> > > > irqsave()
> > > > one) is bad bad bad.
> > >
> > > Hi Stelian.
> > >
> > > I'm sending the new patch without kfifo_put_user and kfifo_get_user.
> > 
> > Ok, I have a few formatting/documentation comments:
> 
> Applied, thanks.
> 
> > Other than that I'm ok with this patch.
> 
> Submitting.
> 
> Regards, Nelson.-
> 
> Signed-off-by: Nelson Castillo <nelson@emqbit.com>

Signed-off-by: Stelian Pop <stelian@popies.net>

Thanks,

Stelian.

-- 
Stelian Pop <stelian@popies.net>