[PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[Stefani Seibold]

This is patch 4/6 of the new kfifo API:
 Add KFIFO_INIT - macro to generate a kfifo initializer
 Add DECLARE_KFIFO - macro to declare a kfifo and the associated buffer inside a struct
 Add INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo 
 Add DEFINE_KFIFO - macro to define and initialize a kfifo as a global or local object
 Add kfifo_size() - returns the size of the fifo in bytes
 Add kfifo_is_empty() - returns true if the fifo is empty
 Add kfifo_is_full() - returns true if the fifo is full
 Add kfifo_avail() - returns the number of bytes available in the FIFO
 Do some code cleanup

 kfifo.h |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 2 deletions(-)

Signed-off-by: Stefani Seibold <stefani@seibold.net>

diff -u -N -r linux-2.6.31-rc4-kfifo4/include/linux/kfifo.h linux-2.6.31-rc4-kfifo5/include/linux/kfifo.h
--- linux-2.6.31-rc4-kfifo4/include/linux/kfifo.h	2009-08-14 10:48:03.000000000 +0200
+++ linux-2.6.31-rc4-kfifo5/include/linux/kfifo.h	2009-08-14 11:53:52.000000000 +0200
@@ -32,6 +32,56 @@
 	unsigned int out;	/* data is extracted from off. (out % size) */
 };
 
+/*
+ * Macros for declaration and initialization of the kfifo datatype
+ */
+
+/**
+ * KFIFO_INIT - macro to generate a kfifo initializer
+ * @s: size of the fifo buffer
+ * @b: address of the fifo buffer
+ */
+#define KFIFO_INIT(s, b) \
+	(struct kfifo) { \
+		.size	= s, \
+		.in	= 0, \
+		.out	= 0, \
+		.buffer = b \
+	}
+
+/**
+ * DECLARE_KFIFO - macro to declare a kfifo and the associated buffer
+ * @name: name of the declared kfifo datatype
+ * @size: size of the fifo buffer
+ *
+ * Note: the macro can be used inside struct or union declaration
+ */
+#define DECLARE_KFIFO(name, size) \
+union { \
+	struct kfifo name; \
+	unsigned char name##_buffer[size + sizeof(struct kfifo)]; \
+}
+
+/**
+ * INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo
+ * @name: name of the declared kfifo datatype
+ * @size: size of the fifo buffer
+ */
+#define INIT_KFIFO(name) \
+	name = KFIFO_INIT(sizeof(name##_buffer) - sizeof(struct kfifo), \
+				name##_buffer)
+
+/**
+ * DEFINE_KFIFO - macro to define and initialize a kfifo
+ * @name: name of the declared kfifo datatype
+ * @size: size of the fifo buffer
+ *
+ * Note: the macro can be used for global and local kfifo data type variables
+ */
+#define DEFINE_KFIFO(name, size) \
+	unsigned char name##_buffer[size]; \
+	struct kfifo name = KFIFO_INIT(size, name##_buffer)
+
 extern void kfifo_init(struct kfifo *fifo,
 			unsigned char *buffer, unsigned int size);
 extern __must_check int kfifo_alloc(struct kfifo *fifo,
@@ -52,6 +102,15 @@
 }
 
 /**
+ * kfifo_size - returns the size of the fifo in bytes
+ * @fifo: the fifo to be used.
+ */
+static inline __must_check unsigned int kfifo_size(struct kfifo *fifo)
+{
+	return fifo->size;
+}
+
+/**
  * kfifo_len - returns the number of used bytes in the FIFO
  * @fifo: the fifo to be used.
  */
@@ -65,6 +124,33 @@
 }
 
 /**
+ * kfifo_is_empty - returns true if the fifo is empty
+ * @fifo: the fifo to be used.
+ */
+static inline __must_check int kfifo_is_empty(struct kfifo *fifo)
+{
+	return fifo->in == fifo->out;
+}
+
+/**
+ * kfifo_is_full - returns true if the fifo is full
+ * @fifo: the fifo to be used.
+ */
+static inline __must_check int kfifo_is_full(struct kfifo *fifo)
+{
+	return kfifo_len(fifo) == kfifo_size(fifo);
+}
+
+/**
+ * kfifo_avail - returns the number of bytes available in the FIFO
+ * @fifo: the fifo to be used.
+ */
+static inline __must_check unsigned int kfifo_avail(struct kfifo *fifo)
+{
+	return kfifo_size(fifo) - kfifo_len(fifo);
+}
+
+/**
  * kfifo_in_locked - puts some data into the FIFO using a spinlock for locking
  * @fifo: the fifo to be used.
  * @from: the data to be added.
@@ -114,8 +200,8 @@
 	 * optimization: if the FIFO is empty, set the indices to 0
 	 * so we don't wrap the next time
 	 */
-	if (fifo->in == fifo->out)
-		fifo->in = fifo->out = 0;
+	if (kfifo_is_empty(fifo))
+		kfifo_reset(fifo);
 
 	spin_unlock_irqrestore(lock, flags);
 

Re: [PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[Arnd Bergmann]

On Friday 14 August 2009, Stefani Seibold wrote:
> Signed-off-by: Stefani Seibold <stefani@seibold.net>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[Joe Perches]

On Fri, 2009-08-14 at 13:44 +0200, Stefani Seibold wrote:

Couple of trivial comments

> This is patch 4/6 of the new kfifo API:
>  Add KFIFO_INIT - macro to generate a kfifo initializer

Is it really necessary to use KFIFO_INIT and INIT_KFIFO?
I think it'll cause confusion and misuse.

> Add INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo 

What does the description mean?

> Add DEFINE_KFIFO - macro to define and initialize a kfifo as a global or local object

What does this mean?  Is scope relevant to the macro?

I think you should mention somewhere that these macros
actually define 2 objects.  "name##_buffer" might have
unexpected clashes and be prefixed with kfifo.
maybe something like "kfifo_##name##_buffer"?

> + * KFIFO_INIT - macro to generate a kfifo initializer
> + * @s: size of the fifo buffer
> + * @b: address of the fifo buffer
> + */
> +#define KFIFO_INIT(s, b) \
> +	(struct kfifo) { \
> +		.size	= s, \
> +		.in	= 0, \
> +		.out	= 0, \
> +		.buffer = b \
> +	}

> +/**
> + * INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo
> + * @name: name of the declared kfifo datatype
> + * @size: size of the fifo buffer
> + */
> +#define INIT_KFIFO(name) \
> +	name = KFIFO_INIT(sizeof(name##_buffer) - sizeof(struct kfifo), \
> +				name##_buffer)

Perhaps

#define __kfifo_initializer ?

Re: [PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[Stefani Seibold]

Am Freitag, den 14.08.2009, 08:20 -0700 schrieb Joe Perches:
> On Fri, 2009-08-14 at 13:44 +0200, Stefani Seibold wrote:
> 
> Couple of trivial comments
> 
> > This is patch 4/6 of the new kfifo API:
> >  Add KFIFO_INIT - macro to generate a kfifo initializer
> 
> Is it really necessary to use KFIFO_INIT and INIT_KFIFO?
> I think it'll cause confusion and misuse.

You are right.

> > + * KFIFO_INIT - macro to generate a kfifo initializer
> > + * @s: size of the fifo buffer
> > + * @b: address of the fifo buffer
> > + */
> > +#define KFIFO_INIT(s, b) \
> > +	(struct kfifo) { \
> > +		.size	= s, \
> > +		.in	= 0, \
> > +		.out	= 0, \
> > +		.buffer = b \
> > +	}
> >
> > +/**
> > + * INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo
> > + * @name: name of the declared kfifo datatype
> > + * @size: size of the fifo buffer
> > + */
> > +#define INIT_KFIFO(name) \
> > +	name = KFIFO_INIT(sizeof(name##_buffer) - sizeof(struct kfifo), \
> > +				name##_buffer)
> 
> Perhaps
> 
> #define __kfifo_initializer ?
> 

I like your idea to rename into __kfifo_initializer. It is only for internal use.

>  
> 
> > Add INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo 
> 
> What does the description mean?

Exactly what it meas. If you declare a kfifo with DECLARE_KIFO than you
must initialize this fifo which INIT_KFIFO.
  
> 
> > Add DEFINE_KFIFO - macro to define and initialize a kfifo as a global or local object
> 
> What does this mean?  Is scope relevant to the macro?
> 

Yes, the scope is relevant, because i found no way to use unnamed unions
for global or local declarations in a way i need it.
 
> I think you should mention somewhere that these macros
> actually define 2 objects.  "name##_buffer" might have
> unexpected clashes and be prefixed with kfifo.
> maybe something like "kfifo_##name##_buffer"?
> 

Maybe name it name##_kfifo_buffer?

But before doing this i will wait for more response and for inclusion
into -mm. If i get an okay i will do a maintainance patch. It is to much
work to handle this splitted patches.

Grettings,
Stefani

Re: [PATCH 4/6] new kfifo API v0.3 - add DEFINE_KFIFO and friends, add very tiny functions

[Joe Perches]

On Fri, 2009-08-14 at 19:03 +0200, Stefani Seibold wrote:
> Am Freitag, den 14.08.2009, 08:20 -0700 schrieb Joe Perches:
> > > Add INIT_KFIFO - macro to initialize a with DECLARE_KFIFO declared kfifo 
> > What does the description mean?
> Exactly what it meas. If you declare a kfifo with DECLARE_KIFO than you
> must initialize this fifo which INIT_KFIFO.

I recognize the intent, but you should read the description to
yourself slowly.  It's also the same content in the kerneldoc.

Maybe INIT_KFIFO - Initialize a kfifo declared by DECLARED_KFIFO

> > I think you should mention somewhere that these macros
> > actually define 2 objects.  "name##_buffer" might have
> > unexpected clashes and be prefixed with kfifo.
> > maybe something like "kfifo_##name##_buffer"?
> Maybe name it name##_kfifo_buffer?

Your choice.  I think most kernel use prefers prefixes.

> But before doing this i will wait for more response and for inclusion
> into -mm. If i get an okay i will do a maintainance patch. It is to much
> work to handle this splitted patches.

Fine by me. cheers, Joe