[PATCH 1/6] dm-bufio: introduce dm_bufio_forget

[PATCH 1/6] dm-bufio: introduce dm_bufio_forget

[Mikulas Patocka]

Introduce a new function dm_bufio_forget. It frees the given buffer.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-bufio.c |   22 ++++++++++++++++++++++
 drivers/md/dm-bufio.h |    7 +++++++
 2 files changed, 29 insertions(+)

Index: linux-3.13-rc7/drivers/md/dm-bufio.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-bufio.c	2014-01-14 00:52:22.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-bufio.c	2014-01-14 00:52:25.000000000 +0100
@@ -1350,6 +1350,28 @@ retry:
 }
 EXPORT_SYMBOL_GPL(dm_bufio_release_move);
 
+/*
+ * Free the given buffer.
+ *
+ * This is just a hint, if the buffer is in use or dirty, this function
+ * does nothing.
+ */
+void dm_bufio_forget(struct dm_bufio_client *c, sector_t block)
+{
+	struct dm_buffer *b;
+
+	dm_bufio_lock(c);
+
+	b = __find(c, block);
+	if (b && likely(!b->hold_count) && likely(!b->state)) {
+		__unlink_buffer(b);
+		__free_buffer_wake(b);
+	}
+
+	dm_bufio_unlock(c);
+}
+EXPORT_SYMBOL(dm_bufio_forget);
+
 unsigned dm_bufio_get_block_size(struct dm_bufio_client *c)
 {
 	return c->block_size;
Index: linux-3.13-rc7/drivers/md/dm-bufio.h
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-bufio.h	2014-01-14 00:52:22.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-bufio.h	2014-01-14 00:52:25.000000000 +0100
@@ -108,6 +108,13 @@ int dm_bufio_issue_flush(struct dm_bufio
  */
 void dm_bufio_release_move(struct dm_buffer *b, sector_t new_block);
 
+/*
+ * Free the given buffer.
+ * This is just a hint, if the buffer is in use or dirty, this function
+ * does nothing.
+ */
+void dm_bufio_forget(struct dm_bufio_client *c, sector_t block);
+
 unsigned dm_bufio_get_block_size(struct dm_bufio_client *c);
 sector_t dm_bufio_get_device_size(struct dm_bufio_client *c);
 sector_t dm_bufio_get_block_number(struct dm_buffer *b);

[PATCH 2/6] dm-bufio: introduce dm_bufio_set_minimum_buffers

[Mikulas Patocka]

The function dm_bufio_set_minimum_buffers sets the number of buffers
before freeing happens. dm-bufio may hold more buffers if enough memory is
available.

There is no guarantee that the specified number of buffers will be
available - if you need a guarantee, use the argument reserved_buffers for
dm_bufio_client_create.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-bufio.c |   14 ++++++++++++--
 drivers/md/dm-bufio.h |    5 +++++
 2 files changed, 17 insertions(+), 2 deletions(-)

Index: linux-3.13-rc7/drivers/md/dm-bufio.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-bufio.c	2014-01-14 00:52:33.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-bufio.c	2014-01-14 00:53:22.000000000 +0100
@@ -104,6 +104,8 @@ struct dm_bufio_client {
 	struct list_head reserved_buffers;
 	unsigned need_reserved_buffers;
 
+	unsigned minimum_buffers;
+
 	struct hlist_head *cache_hash;
 	wait_queue_head_t free_buffer_wait;
 
@@ -861,8 +863,8 @@ static void __get_memory_limit(struct dm
 	buffers = dm_bufio_cache_size_per_client >>
 		  (c->sectors_per_block_bits + SECTOR_SHIFT);
 
-	if (buffers < DM_BUFIO_MIN_BUFFERS)
-		buffers = DM_BUFIO_MIN_BUFFERS;
+	if (buffers < c->minimum_buffers)
+		buffers = c->minimum_buffers;
 
 	*limit_buffers = buffers;
 	*threshold_buffers = buffers * DM_BUFIO_WRITEBACK_PERCENT / 100;
@@ -1372,6 +1374,12 @@ void dm_bufio_forget(struct dm_bufio_cli
 }
 EXPORT_SYMBOL(dm_bufio_forget);
 
+void dm_bufio_set_minimum_buffers(struct dm_bufio_client *c, unsigned n)
+{
+	c->minimum_buffers = n;
+}
+EXPORT_SYMBOL(dm_bufio_set_minimum_buffers);
+
 unsigned dm_bufio_get_block_size(struct dm_bufio_client *c)
 {
 	return c->block_size;
@@ -1568,6 +1576,8 @@ struct dm_bufio_client *dm_bufio_client_
 	INIT_LIST_HEAD(&c->reserved_buffers);
 	c->need_reserved_buffers = reserved_buffers;
 
+	c->minimum_buffers = DM_BUFIO_MIN_BUFFERS;
+
 	init_waitqueue_head(&c->free_buffer_wait);
 	c->async_write_error = 0;
 
Index: linux-3.13-rc7/drivers/md/dm-bufio.h
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-bufio.h	2014-01-14 00:52:34.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-bufio.h	2014-01-14 00:53:22.000000000 +0100
@@ -115,6 +115,11 @@ void dm_bufio_release_move(struct dm_buf
  */
 void dm_bufio_forget(struct dm_bufio_client *c, sector_t block);
 
+/*
+ * Set the minimum number of buffers before cleanup happens.
+ */
+void dm_bufio_set_minimum_buffers(struct dm_bufio_client *c, unsigned n);
+
 unsigned dm_bufio_get_block_size(struct dm_bufio_client *c);
 sector_t dm_bufio_get_device_size(struct dm_bufio_client *c);
 sector_t dm_bufio_get_block_number(struct dm_buffer *b);

[PATCH 3/6] dm-snapshot: use GFP_KERNEL when initializing the snapshot

[Mikulas Patocka]

The list of initial exceptions is loaded in the target constructor. We are
allowed to allocate memory with GFP_KERNEL at this point. So, change
alloc_completed_exception to use GFP_KERNEL when being called from the
constructor.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-snap.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-3.13-rc7/drivers/md/dm-snap.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-snap.c	2014-01-09 20:42:51.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-snap.c	2014-01-09 20:43:41.000000000 +0100
@@ -610,12 +610,12 @@ static struct dm_exception *dm_lookup_ex
 	return NULL;
 }
 
-static struct dm_exception *alloc_completed_exception(void)
+static struct dm_exception *alloc_completed_exception(gfp_t gfp)
 {
 	struct dm_exception *e;
 
-	e = kmem_cache_alloc(exception_cache, GFP_NOIO);
-	if (!e)
+	e = kmem_cache_alloc(exception_cache, gfp);
+	if (!e && gfp == GFP_NOIO)
 		e = kmem_cache_alloc(exception_cache, GFP_ATOMIC);
 
 	return e;
@@ -697,7 +697,7 @@ static int dm_add_exception(void *contex
 	struct dm_snapshot *s = context;
 	struct dm_exception *e;
 
-	e = alloc_completed_exception();
+	e = alloc_completed_exception(GFP_KERNEL);
 	if (!e)
 		return -ENOMEM;
 
@@ -1405,7 +1405,7 @@ static void pending_complete(struct dm_s
 		goto out;
 	}
 
-	e = alloc_completed_exception();
+	e = alloc_completed_exception(GFP_NOIO);
 	if (!e) {
 		down_write(&s->lock);
 		__invalidate_snapshot(s, -ENOMEM);

[PATCH 4/6] dm-snapshot: make ps->area explicit

[Mikulas Patocka]

This patch changes the functions get_exception, read_exception and
insert_exceptions so that the pointer ps->area is given as an argument.

This patch doesn't change any functionality, but it refactors the code for
the next patch.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-snap-persistent.c |   26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

Index: linux-3.13-rc7/drivers/md/dm-snap-persistent.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-snap-persistent.c	2014-01-09 23:37:59.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-snap-persistent.c	2014-01-09 23:40:22.000000000 +0100
@@ -401,17 +401,18 @@ static int write_header(struct pstore *p
 /*
  * Access functions for the disk exceptions, these do the endian conversions.
  */
-static struct disk_exception *get_exception(struct pstore *ps, uint32_t index)
+static struct disk_exception *get_exception(struct pstore *ps, void *ps_area,
+					    uint32_t index)
 {
 	BUG_ON(index >= ps->exceptions_per_area);
 
-	return ((struct disk_exception *) ps->area) + index;
+	return ((struct disk_exception *) ps_area) + index;
 }
 
-static void read_exception(struct pstore *ps,
+static void read_exception(struct pstore *ps, void *ps_area,
 			   uint32_t index, struct core_exception *result)
 {
-	struct disk_exception *de = get_exception(ps, index);
+	struct disk_exception *de = get_exception(ps, ps_area, index);
 
 	/* copy it */
 	result->old_chunk = le64_to_cpu(de->old_chunk);
@@ -421,7 +422,7 @@ static void read_exception(struct pstore
 static void write_exception(struct pstore *ps,
 			    uint32_t index, struct core_exception *e)
 {
-	struct disk_exception *de = get_exception(ps, index);
+	struct disk_exception *de = get_exception(ps, ps->area, index);
 
 	/* copy it */
 	de->old_chunk = cpu_to_le64(e->old_chunk);
@@ -430,7 +431,7 @@ static void write_exception(struct pstor
 
 static void clear_exception(struct pstore *ps, uint32_t index)
 {
-	struct disk_exception *de = get_exception(ps, index);
+	struct disk_exception *de = get_exception(ps, ps->area, index);
 
 	/* clear it */
 	de->old_chunk = 0;
@@ -442,7 +443,7 @@ static void clear_exception(struct pstor
  * 'full' is filled in to indicate if the area has been
  * filled.
  */
-static int insert_exceptions(struct pstore *ps,
+static int insert_exceptions(struct pstore *ps, void *ps_area,
 			     int (*callback)(void *callback_context,
 					     chunk_t old, chunk_t new),
 			     void *callback_context,
@@ -456,7 +457,7 @@ static int insert_exceptions(struct psto
 	*full = 1;
 
 	for (i = 0; i < ps->exceptions_per_area; i++) {
-		read_exception(ps, i, &e);
+		read_exception(ps, ps_area, i, &e);
 
 		/*
 		 * If the new_chunk is pointing at the start of
@@ -503,7 +504,8 @@ static int read_exceptions(struct pstore
 		if (r)
 			return r;
 
-		r = insert_exceptions(ps, callback, callback_context, &full);
+		r = insert_exceptions(ps, ps->area, callback, callback_context,
+				      &full);
 		if (r)
 			return r;
 	}
@@ -733,7 +735,7 @@ static int persistent_prepare_merge(stru
 		ps->current_committed = ps->exceptions_per_area;
 	}
 
-	read_exception(ps, ps->current_committed - 1, &ce);
+	read_exception(ps, ps->area, ps->current_committed - 1, &ce);
 	*last_old_chunk = ce.old_chunk;
 	*last_new_chunk = ce.new_chunk;
 
@@ -743,8 +745,8 @@ static int persistent_prepare_merge(stru
 	 */
 	for (nr_consecutive = 1; nr_consecutive < ps->current_committed;
 	     nr_consecutive++) {
-		read_exception(ps, ps->current_committed - 1 - nr_consecutive,
-			       &ce);
+		read_exception(ps, ps->area,
+			       ps->current_committed - 1 - nr_consecutive, &ce);
 		if (ce.old_chunk != *last_old_chunk - nr_consecutive ||
 		    ce.new_chunk != *last_new_chunk - nr_consecutive)
 			break;

[PATCH 5/6] dm-snapshot: use dm-bufio

[Mikulas Patocka]

Use dm-bufio for initial loading of the exceptions.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-snap-persistent.c |   39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

Index: linux-3.13-rc7/drivers/md/dm-snap-persistent.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-snap-persistent.c	2014-01-10 03:08:36.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-snap-persistent.c	2014-01-10 03:40:56.000000000 +0100
@@ -13,6 +13,7 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/dm-io.h>
+#include "dm-bufio.h"
 
 #define DM_MSG_PREFIX "persistent snapshot"
 #define DM_CHUNK_SIZE_DEFAULT_SECTORS 32	/* 16KB */
@@ -494,27 +495,51 @@ static int read_exceptions(struct pstore
 			   void *callback_context)
 {
 	int r, full = 1;
+	struct dm_bufio_client *client;
+
+	client = dm_bufio_client_create(dm_snap_cow(ps->store->snap)->bdev,
+					ps->store->chunk_size << SECTOR_SHIFT,
+					1, 0, NULL, NULL);
+
+	if (IS_ERR(client))
+		return PTR_ERR(client);
 
 	/*
 	 * Keeping reading chunks and inserting exceptions until
 	 * we find a partially full area.
 	 */
 	for (ps->current_area = 0; full; ps->current_area++) {
-		r = area_io(ps, READ);
-		if (r)
-			return r;
+		struct dm_buffer *bp;
+		void *area;
+		chunk_t chunk = area_location(ps, ps->current_area);
+
+		area = dm_bufio_read(client, chunk, &bp);
+		if (unlikely(IS_ERR(area))) {
+			r = PTR_ERR(area);
+			goto ret_destroy_bufio;
+		}
 
-		r = insert_exceptions(ps, ps->area, callback, callback_context,
+		r = insert_exceptions(ps, area, callback, callback_context,
 				      &full);
-		if (r)
-			return r;
+
+		dm_bufio_release(bp);
+
+		dm_bufio_forget(client, chunk);
+
+		if (unlikely(r))
+			goto ret_destroy_bufio;
 	}
 
 	ps->current_area--;
 
 	skip_metadata(ps);
 
-	return 0;
+	r = 0;
+
+ret_destroy_bufio:
+	dm_bufio_client_destroy(client);
+
+	return r;
 }
 
 static struct pstore *get_info(struct dm_exception_store *store)

[PATCH 6/6] dm-snapshot: use bufio prefetch

[Mikulas Patocka]

This patch modifies dm-snapshot so that it prefetches the buffers when
loading the exceptions.

The number of buffers read ahead is specified in the DM_PREFETCH_CHUNKS
macro.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-snap-persistent.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Index: linux-3.13-rc7/drivers/md/dm-snap-persistent.c
===================================================================
--- linux-3.13-rc7.orig/drivers/md/dm-snap-persistent.c	2014-01-14 00:54:52.000000000 +0100
+++ linux-3.13-rc7/drivers/md/dm-snap-persistent.c	2014-01-14 00:56:39.000000000 +0100
@@ -18,6 +18,8 @@
 #define DM_MSG_PREFIX "persistent snapshot"
 #define DM_CHUNK_SIZE_DEFAULT_SECTORS 32	/* 16KB */
 
+#define DM_PREFETCH_CHUNKS		12
+
 /*-----------------------------------------------------------------
  * Persistent snapshots, by persistent we mean that the snapshot
  * will survive a reboot.
@@ -496,6 +498,7 @@ static int read_exceptions(struct pstore
 {
 	int r, full = 1;
 	struct dm_bufio_client *client;
+	chunk_t prefetch_area = 0;
 
 	client = dm_bufio_client_create(dm_snap_cow(ps->store->snap)->bdev,
 					ps->store->chunk_size << SECTOR_SHIFT,
@@ -504,6 +507,8 @@ static int read_exceptions(struct pstore
 	if (IS_ERR(client))
 		return PTR_ERR(client);
 
+	dm_bufio_set_minimum_buffers(client, DM_PREFETCH_CHUNKS + 1);
+
 	/*
 	 * Keeping reading chunks and inserting exceptions until
 	 * we find a partially full area.
@@ -511,7 +516,22 @@ static int read_exceptions(struct pstore
 	for (ps->current_area = 0; full; ps->current_area++) {
 		struct dm_buffer *bp;
 		void *area;
-		chunk_t chunk = area_location(ps, ps->current_area);
+		chunk_t chunk;
+
+		if (unlikely(prefetch_area < ps->current_area))
+			prefetch_area = ps->current_area;
+
+		if (DM_PREFETCH_CHUNKS) do {
+			chunk_t pf_chunk = area_location(ps, prefetch_area);
+			if (unlikely(pf_chunk >= dm_bufio_get_device_size(client)))
+				break;
+			dm_bufio_prefetch(client, pf_chunk, 1);
+			prefetch_area++;
+			if (unlikely(!prefetch_area))
+				break;
+		} while (prefetch_area <= ps->current_area + DM_PREFETCH_CHUNKS);
+
+		chunk = area_location(ps, ps->current_area);
 
 		area = dm_bufio_read(client, chunk, &bp);
 		if (unlikely(IS_ERR(area))) {

Re: [PATCH 6/6] dm-snapshot: use bufio prefetch

[Mike Snitzer]

On Mon, Jan 13 2014 at  7:14pm -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> This patch modifies dm-snapshot so that it prefetches the buffers when
> loading the exceptions.
> 
> The number of buffers read ahead is specified in the DM_PREFETCH_CHUNKS
> macro.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/dm-snap-persistent.c |   22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> Index: linux-3.13-rc7/drivers/md/dm-snap-persistent.c
> ===================================================================
> --- linux-3.13-rc7.orig/drivers/md/dm-snap-persistent.c	2014-01-14 00:54:52.000000000 +0100
> +++ linux-3.13-rc7/drivers/md/dm-snap-persistent.c	2014-01-14 00:56:39.000000000 +0100
> @@ -18,6 +18,8 @@
>  #define DM_MSG_PREFIX "persistent snapshot"
>  #define DM_CHUNK_SIZE_DEFAULT_SECTORS 32	/* 16KB */
>  
> +#define DM_PREFETCH_CHUNKS		12
> +
>  /*-----------------------------------------------------------------
>   * Persistent snapshots, by persistent we mean that the snapshot
>   * will survive a reboot.
...
> @@ -504,6 +507,8 @@ static int read_exceptions(struct pstore
>  	if (IS_ERR(client))
>  		return PTR_ERR(client);
>  
> +	dm_bufio_set_minimum_buffers(client, DM_PREFETCH_CHUNKS + 1);
> +

Why the +1?  I'd prefer that be managed in dm-bufio rather than relying
on all callers to know to add +1.

Can you supply a followup patch that cleans this up in bufio and remove
the +1 in the above dm_bufio_set_minimum_buffers call?

Re: [PATCH 6/6] dm-snapshot: use bufio prefetch

[Mikulas Patocka]

On Tue, 14 Jan 2014, Mike Snitzer wrote:

> On Mon, Jan 13 2014 at  7:14pm -0500,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > +	dm_bufio_set_minimum_buffers(client, DM_PREFETCH_CHUNKS + 1);
> > +
> 
> Why the +1?  I'd prefer that be managed in dm-bufio rather than relying
> on all callers to know to add +1.
> 
> Can you supply a followup patch that cleans this up in bufio and remove
> the +1 in the above dm_bufio_set_minimum_buffers call?

If DM_PREFETCH_CHUNKS is 12, we need 13 buffers in dm-bufio (one for the 
current buffer that is being read and 12 read ahead buffers).

It is correct to have +1 in snapshot code, not in dm-bufio code.

Mikulas

Re: [PATCH 6/6] dm-snapshot: use bufio prefetch

[Mike Snitzer]

On Tue, Jan 14 2014 at  3:43pm -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Tue, 14 Jan 2014, Mike Snitzer wrote:
> 
> > On Mon, Jan 13 2014 at  7:14pm -0500,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > 
> > > +	dm_bufio_set_minimum_buffers(client, DM_PREFETCH_CHUNKS + 1);
> > > +
> > 
> > Why the +1?  I'd prefer that be managed in dm-bufio rather than relying
> > on all callers to know to add +1.
> > 
> > Can you supply a followup patch that cleans this up in bufio and remove
> > the +1 in the above dm_bufio_set_minimum_buffers call?
> 
> If DM_PREFETCH_CHUNKS is 12, we need 13 buffers in dm-bufio (one for the 
> current buffer that is being read and 12 read ahead buffers).
> 
> It is correct to have +1 in snapshot code, not in dm-bufio code.

OK, thanks for clarifying.

I've folded the dm bufio changes needed for the respective dm snapshot
changes -- so there are now 4 patches instead of your otiginal 6.
I added a comment for "1 + DM_PREFETCH_CHUNKS" above the
dm_bufio_set_minimum_buffers call and tweaked the headers a little.

I also staged your non-modular kobject release patch (tweaked the header
on that a little bit too).

Please see the 5 topmost commits here:
http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next

Re: [PATCH 6/6] dm-snapshot: use bufio prefetch

[Mikulas Patocka]

On Tue, 14 Jan 2014, Mike Snitzer wrote:

> On Tue, Jan 14 2014 at  3:43pm -0500,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > 
> > 
> > On Tue, 14 Jan 2014, Mike Snitzer wrote:
> > 
> > > On Mon, Jan 13 2014 at  7:14pm -0500,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > 
> > > > +	dm_bufio_set_minimum_buffers(client, DM_PREFETCH_CHUNKS + 1);
> > > > +
> > > 
> > > Why the +1?  I'd prefer that be managed in dm-bufio rather than relying
> > > on all callers to know to add +1.
> > > 
> > > Can you supply a followup patch that cleans this up in bufio and remove
> > > the +1 in the above dm_bufio_set_minimum_buffers call?
> > 
> > If DM_PREFETCH_CHUNKS is 12, we need 13 buffers in dm-bufio (one for the 
> > current buffer that is being read and 12 read ahead buffers).
> > 
> > It is correct to have +1 in snapshot code, not in dm-bufio code.
> 
> OK, thanks for clarifying.
> 
> I've folded the dm bufio changes needed for the respective dm snapshot
> changes -- so there are now 4 patches instead of your otiginal 6.
> I added a comment for "1 + DM_PREFETCH_CHUNKS" above the
> dm_bufio_set_minimum_buffers call and tweaked the headers a little.
> 
> I also staged your non-modular kobject release patch (tweaked the header
> on that a little bit too).
> 
> Please see the 5 topmost commits here:
> http://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/log/?h=for-next

OK. The patches look good.

Mikulas