2008-03-24 15:08:30

by Nitin Gupta

[permalink] [raw]
Subject: [PATCH 2/6] compcache: block device - internal defs

This contains header to be used internally by block device code.
It contains flags to enable/disable debugging, stats collection and also
defines default disk size (25% of total RAM).

Signed-off-by: Nitin Gupta <nitingupta910 at gmail dot com>
---
drivers/block/compcache.h | 147 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 147 insertions(+), 0 deletions(-)

diff --git a/drivers/block/compcache.h b/drivers/block/compcache.h
new file mode 100644
index 0000000..b84b5d3
--- /dev/null
+++ b/drivers/block/compcache.h
@@ -0,0 +1,147 @@
+/*
+ * Compressed RAM based swap device
+ *
+ * (C) Nitin Gupta
+ *
+ * This RAM based block device acts as swap disk.
+ * Pages swapped to this device are compressed and
+ * stored in memory.
+ *
+ * Project home: http://code.google.com/p/compcache
+ */
+
+#ifndef _COMPCACHE_H_
+#define _COMPCACHE_H_
+
+#define K(x) ((x) >> 10)
+#define KB(x) ((x) << 10)
+
+#define SECTOR_SHIFT 9
+#define SECTOR_SIZE (1 << SECTOR_SHIFT)
+#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
+#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
+
+/*-- Configurable parameters */
+/* Default compcache size: 25% of total RAM */
+#define DEFAULT_COMPCACHE_PERCENT 25
+#define INIT_SIZE KB(16)
+#define GROW_SIZE INIT_SIZE
+/*-- */
+
+/* Message prefix */
+#define C "compcache: "
+
+/* Debugging and Stats */
+#define NOP do { } while(0)
+
+#if (1 || defined(CONFIG_DEBUG_COMPCACHE))
+#define DEBUG 1
+#define STATS 1
+#else
+#define DEBUG 0
+#define STATS 0
+#endif
+
+/* Create /proc/compcache? */
+/* If STATS is disabled, this will give minimal compcache info */
+#define CONFIG_COMPCACHE_PROC
+
+#if DEBUG
+#define CC_DEBUG(fmt,arg...) \
+ printk(KERN_DEBUG C fmt,##arg)
+#else
+#define CC_DEBUG(fmt,arg...) NOP
+#endif
+
+/*
+ * Verbose debugging:
+ * Enable basic debugging + verbose messages spread all over code
+ */
+#define DEBUG2 0
+
+#if DEBUG2
+#define DEBUG 1
+#define STATS 1
+#define CONFIG_COMPCACHE_PROC 1
+#define CC_DEBUG2((fmt,arg...) \
+ printk(KERN_DEBUG C fmt,##arg)
+#else /* DEBUG2 */
+#define CC_DEBUG2(fmt,arg...) NOP
+#endif
+
+/* Its useless to collect stats if there is no way to export it */
+#if (STATS && !defined(CONFIG_COMPCACHE_PROC))
+#error "compcache stats is enabled but not /proc/compcache."
+#endif
+
+#if STATS
+static inline void stat_inc_if_less(size_t *stat, const size_t val1,
+ const size_t val2)
+{
+ *stat += ((val1 < val2) ? 1 : 0);
+}
+
+static inline void stat_inc(size_t *stat)
+{
+ ++*stat;
+}
+
+static inline void stat_dec(size_t *stat)
+{
+ BUG_ON(*stat == 0);
+ --*stat;
+}
+
+static inline void stat_set(size_t *stat, const size_t val)
+{
+ *stat = val;
+}
+
+static inline void stat_setmax(size_t *max, const size_t cur)
+{
+ *max = (cur > *max) ? cur : *max;
+}
+#else /* STATS */
+#define stat_inc(x) NOP
+#define stat_dec(x) NOP
+#define stat_set(x, v) NOP
+#define stat_setmax(x, v) NOP
+#define stat_inc_if_less(x, v1, v2) NOP
+#endif /* STATS */
+
+/*-- Data structures */
+/* Indexed by page no. */
+struct table {
+ void *addr;
+ unsigned short len;
+} __attribute__ ((packed));
+
+struct compcache {
+ void *mem_pool;
+ void *compress_workmem;
+ void *compress_buffer;
+ struct table *table;
+ struct mutex lock;
+ struct gendisk *disk;
+ size_t size; /* In sectors */
+};
+
+#if STATS
+struct compcache_stats {
+ u32 num_reads; /* failed + successful */
+ u32 num_writes; /* --do-- */
+ u32 failed_reads; /* can happen when memory is tooo low */
+ u32 failed_writes; /* should NEVER! happen */
+ u32 invalid_io; /* non-swap I/O requests */
+ u32 good_compress; /* no. of pages with compression
+ * ratio <= 50%. TODO: export full
+ * compressed page size histogram */
+ u32 pages_expand; /* no. of incompressible pages */
+ size_t curr_pages; /* current no. of compressed pages */
+ size_t curr_mem; /* current total size of compressed pages */
+ size_t peak_mem;
+};
+#endif
+/*-- */
+
+#endif


2008-03-24 16:05:29

by Will Newton

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

On Mon, Mar 24, 2008 at 3:03 PM, Nitin Gupta <[email protected]> wrote:

Hi Nitin,

> This contains header to be used internally by block device code.
> It contains flags to enable/disable debugging, stats collection and also
> defines default disk size (25% of total RAM).
>
> Signed-off-by: Nitin Gupta <nitingupta910 at gmail dot com>
> ---
> drivers/block/compcache.h | 147 +++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 147 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/block/compcache.h b/drivers/block/compcache.h
> new file mode 100644
> index 0000000..b84b5d3
> --- /dev/null
> +++ b/drivers/block/compcache.h
> @@ -0,0 +1,147 @@
> +/*
> + * Compressed RAM based swap device
> + *
> + * (C) Nitin Gupta
> + *
> + * This RAM based block device acts as swap disk.
> + * Pages swapped to this device are compressed and
> + * stored in memory.
> + *
> + * Project home: http://code.google.com/p/compcache
> + */
> +
> +#ifndef _COMPCACHE_H_
> +#define _COMPCACHE_H_
> +
> +#define K(x) ((x) >> 10)
> +#define KB(x) ((x) << 10)
> +
> +#define SECTOR_SHIFT 9
> +#define SECTOR_SIZE (1 << SECTOR_SHIFT)
> +#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
> +#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
> +
> +/*-- Configurable parameters */
> +/* Default compcache size: 25% of total RAM */
> +#define DEFAULT_COMPCACHE_PERCENT 25
> +#define INIT_SIZE KB(16)
> +#define GROW_SIZE INIT_SIZE

Maybe these could be renamed to INIT_SIZE_BYTES/GROW_SIZE_BYTES to
make the units clearer?

> +/*-- */
> +
> +/* Message prefix */
> +#define C "compcache: "
> +
> +/* Debugging and Stats */
> +#define NOP do { } while(0)
> +
> +#if (1 || defined(CONFIG_DEBUG_COMPCACHE))
> +#define DEBUG 1
> +#define STATS 1
> +#else
> +#define DEBUG 0
> +#define STATS 0
> +#endif

If DEBUG is defined unconditionally what is the point of CONFIG_DEBUG_COMPCACHE?

> +
> +/* Create /proc/compcache? */
> +/* If STATS is disabled, this will give minimal compcache info */
> +#define CONFIG_COMPCACHE_PROC
> +
> +#if DEBUG
> +#define CC_DEBUG(fmt,arg...) \
> + printk(KERN_DEBUG C fmt,##arg)
> +#else
> +#define CC_DEBUG(fmt,arg...) NOP
> +#endif

Have you thought about using pr_debug() for this? It looks like it
would simplify this file at the cost of a little flexibility.

> +
> +/*
> + * Verbose debugging:
> + * Enable basic debugging + verbose messages spread all over code
> + */
> +#define DEBUG2 0
> +
> +#if DEBUG2
> +#define DEBUG 1
> +#define STATS 1
> +#define CONFIG_COMPCACHE_PROC 1
> +#define CC_DEBUG2((fmt,arg...) \
> + printk(KERN_DEBUG C fmt,##arg)
> +#else /* DEBUG2 */
> +#define CC_DEBUG2(fmt,arg...) NOP
> +#endif
> +
> +/* Its useless to collect stats if there is no way to export it */
> +#if (STATS && !defined(CONFIG_COMPCACHE_PROC))
> +#error "compcache stats is enabled but not /proc/compcache."
> +#endif

So it appears that if we want DEBUG we also get STATS, which requires
/proc support enabled, so it is impossible to have just DEBUG and no
STATS or /proc support?

2008-03-24 16:26:16

by Mariusz Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

Hi Nitin,

> +#define K(x) ((x) >> 10)
> +#define KB(x) ((x) << 10)

Hm. These look cryptic unless you remember what they do.
Could have better names?

> +#define CC_DEBUG2((fmt,arg...) \
> +???????printk(KERN_DEBUG C fmt,##arg)

Unbalanced parenthesis.

Just my 0.05zl.

Mariusz

2008-03-24 17:39:56

by Nitin Gupta

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

On Mon, Mar 24, 2008 at 9:55 PM, Mariusz Kozlowski
<[email protected]> wrote:
> Hi Nitin,
>
>
> > +#define K(x) ((x) >> 10)
> > +#define KB(x) ((x) << 10)
>
> Hm. These look cryptic unless you remember what they do.
> Could have better names?

I'll give them better names/add comments.

>
>
> > +#define CC_DEBUG2((fmt,arg...) \
> > + printk(KERN_DEBUG C fmt,##arg)
>
> Unbalanced parenthesis.
>

Corrected. Thanks.

- Nitin

> Just my 0.05zl.
>
> Mariusz
>

2008-03-24 17:51:18

by Nitin Gupta

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

On Mon, Mar 24, 2008 at 9:35 PM, Will Newton <[email protected]> wrote:
> On Mon, Mar 24, 2008 at 3:03 PM, Nitin Gupta <[email protected]> wrote:
>
> Hi Nitin,
>
>
>
> > This contains header to be used internally by block device code.
> > It contains flags to enable/disable debugging, stats collection and also
> > defines default disk size (25% of total RAM).
> >
> > Signed-off-by: Nitin Gupta <nitingupta910 at gmail dot com>
> > ---
> > drivers/block/compcache.h | 147 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 147 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/block/compcache.h b/drivers/block/compcache.h
> > new file mode 100644
> > index 0000000..b84b5d3
> > --- /dev/null
> > +++ b/drivers/block/compcache.h
> > @@ -0,0 +1,147 @@
> > +/*
> > + * Compressed RAM based swap device
> > + *
> > + * (C) Nitin Gupta
> > + *
> > + * This RAM based block device acts as swap disk.
> > + * Pages swapped to this device are compressed and
> > + * stored in memory.
> > + *
> > + * Project home: http://code.google.com/p/compcache
> > + */
> > +
> > +#ifndef _COMPCACHE_H_
> > +#define _COMPCACHE_H_
> > +
> > +#define K(x) ((x) >> 10)
> > +#define KB(x) ((x) << 10)
> > +
> > +#define SECTOR_SHIFT 9
> > +#define SECTOR_SIZE (1 << SECTOR_SHIFT)
> > +#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
> > +#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
> > +
> > +/*-- Configurable parameters */
> > +/* Default compcache size: 25% of total RAM */
> > +#define DEFAULT_COMPCACHE_PERCENT 25
> > +#define INIT_SIZE KB(16)
> > +#define GROW_SIZE INIT_SIZE
>
> Maybe these could be renamed to INIT_SIZE_BYTES/GROW_SIZE_BYTES to
> make the units clearer?
>


Sounds better. I will rename these.


>
> > +/*-- */
> > +
> > +/* Message prefix */
> > +#define C "compcache: "
> > +
> > +/* Debugging and Stats */
> > +#define NOP do { } while(0)
> > +
> > +#if (1 || defined(CONFIG_DEBUG_COMPCACHE))
> > +#define DEBUG 1
> > +#define STATS 1
> > +#else
> > +#define DEBUG 0
> > +#define STATS 0
> > +#endif
>
> If DEBUG is defined unconditionally what is the point of CONFIG_DEBUG_COMPCACHE?
>
>

For now I have not added DEBUG_COMPCACHE as make option (menuconfig)
so its unconditional for now. I am now adding this option and then it
will not be unconditional.

> > +
> > +/* Create /proc/compcache? */
> > +/* If STATS is disabled, this will give minimal compcache info */
> > +#define CONFIG_COMPCACHE_PROC
> > +
> > +#if DEBUG
> > +#define CC_DEBUG(fmt,arg...) \
> > + printk(KERN_DEBUG C fmt,##arg)
> > +#else
> > +#define CC_DEBUG(fmt,arg...) NOP
> > +#endif
>
> Have you thought about using pr_debug() for this? It looks like it
> would simplify this file at the cost of a little flexibility.
>

I want to enable/disable this debugging based on DEBUG_COMPCACHE flag.
Thats why I added these macros. I will do 'printk(KERN_DEBUG' ->
pr_debug


>
> > +
> > +/*
> > + * Verbose debugging:
> > + * Enable basic debugging + verbose messages spread all over code
> > + */
> > +#define DEBUG2 0
> > +
> > +#if DEBUG2
> > +#define DEBUG 1
> > +#define STATS 1
> > +#define CONFIG_COMPCACHE_PROC 1
> > +#define CC_DEBUG2((fmt,arg...) \
> > + printk(KERN_DEBUG C fmt,##arg)
> > +#else /* DEBUG2 */
> > +#define CC_DEBUG2(fmt,arg...) NOP
> > +#endif
> > +
> > +/* Its useless to collect stats if there is no way to export it */
> > +#if (STATS && !defined(CONFIG_COMPCACHE_PROC))
> > +#error "compcache stats is enabled but not /proc/compcache."
> > +#endif
>
> So it appears that if we want DEBUG we also get STATS, which requires
> /proc support enabled, so it is impossible to have just DEBUG and no
> STATS or /proc support?
>

I will decouple debug and stats so each can be enabled/disabled individually.

Thanks,
Nitin

2008-03-24 20:36:17

by Will Newton

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

On Mon, Mar 24, 2008 at 5:50 PM, Nitin Gupta <[email protected]> wrote:
> > > +
> > > +/* Create /proc/compcache? */
> > > +/* If STATS is disabled, this will give minimal compcache info */
> > > +#define CONFIG_COMPCACHE_PROC
> > > +
> > > +#if DEBUG
> > > +#define CC_DEBUG(fmt,arg...) \
> > > + printk(KERN_DEBUG C fmt,##arg)
> > > +#else
> > > +#define CC_DEBUG(fmt,arg...) NOP
> > > +#endif
> >
> > Have you thought about using pr_debug() for this? It looks like it
> > would simplify this file at the cost of a little flexibility.
> >
>
> I want to enable/disable this debugging based on DEBUG_COMPCACHE flag.
> Thats why I added these macros. I will do 'printk(KERN_DEBUG' ->
> pr_debug

The definition of pr_debug (kernel.h) is already surrounded by #ifdef
DEBUG so it may give you the same behaviour as the CC_DEBUG macro.

2008-03-24 20:44:22

by Nitin Gupta

[permalink] [raw]
Subject: Re: [PATCH 2/6] compcache: block device - internal defs

On Tuesday 25 March 2008 02:06:02 am Will Newton wrote:
> On Mon, Mar 24, 2008 at 5:50 PM, Nitin Gupta <[email protected]> wrote:
> > > > +
> > > > +/* Create /proc/compcache? */
> > > > +/* If STATS is disabled, this will give minimal compcache info */
> > > > +#define CONFIG_COMPCACHE_PROC
> > > > +
> > > > +#if DEBUG
> > > > +#define CC_DEBUG(fmt,arg...) \
> > > > + printk(KERN_DEBUG C fmt,##arg)
> > > > +#else
> > > > +#define CC_DEBUG(fmt,arg...) NOP
> > > > +#endif
> > >
> > > Have you thought about using pr_debug() for this? It looks like it
> > > would simplify this file at the cost of a little flexibility.
> > >
> >
> > I want to enable/disable this debugging based on DEBUG_COMPCACHE flag.
> > Thats why I added these macros. I will do 'printk(KERN_DEBUG' ->
> > pr_debug
>
> The definition of pr_debug (kernel.h) is already surrounded by #ifdef
> DEBUG so it may give you the same behaviour as the CC_DEBUG macro.
>

Yes, I missed this point. But still, I want to have two levels of debugging. I can probably use pr_debug() for "normal" debug and CC_DEBUG for "verbose" debugging. This looks bit inconsistent, so maybe I should stick which CC_DEBUG/CC_DEBUG2 pair instead?

- Nitin