labels: avoid mda_header read using async read data

Extend the initial async read buffer size to cover all
the headers/metadata that need to be read from the device
during label scan.

Copy the mda_header from this buffer instead of performing
another synchronous read for it.

7 files changed, 92 insertions, 48 deletions

diff --git a/lib/format1/lvm1-label.c b/lib/format1/lvm1-label.c
index f5ce1e892..a9ccaaf6b 100644
--- a/lib/format1/lvm1-label.c
+++ b/lib/format1/lvm1-label.c
@@ -55,6 +55,7 @@ static int _lvm1_write(struct label *label __attribute__((unused)), void *buf __
 }
 
 static int _lvm1_read(struct labeller *l, struct device *dev, void *buf,
+		 struct label_read_data *ld,
 		 struct label **label)
 {
 	struct pv_disk *pvd = (struct pv_disk *) buf;
diff --git a/lib/format_pool/pool_label.c b/lib/format_pool/pool_label.c
index 0f1d1f791..fa34c2fe1 100644
--- a/lib/format_pool/pool_label.c
+++ b/lib/format_pool/pool_label.c
@@ -56,6 +56,7 @@ static int _pool_write(struct label *label __attribute__((unused)), void *buf __
 }
 
 static int _pool_read(struct labeller *l, struct device *dev, void *buf,
+		 struct label_read_data *ld,
 		 struct label **label)
 {
 	struct pool_list pl;
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 85c773b3c..123489749 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -190,7 +190,7 @@ static int _pv_analyze_mda_raw (const struct format_type * fmt,
 	if (!dev_open_readonly(area->dev))
 		return_0;
 
-	if (!(mdah = raw_read_mda_header(fmt, area)))
+	if (!(mdah = raw_read_mda_header(fmt, area, NULL)))
 		goto_out;
 
 	rlocn = mdah->raw_locns;
@@ -316,18 +316,26 @@ static void _xlate_mdah(struct mda_header *mdah)
 	}
 }
 
-static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area)
+static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area,
+				struct label_read_data *ld)
 {
 	if (!dev_open_readonly(dev_area->dev))
 		return_0;
 
-	log_debug_metadata("Reading mda header sector from %s at %llu",
-			   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
+	if (!ld || (ld->buf_len < dev_area->start + MDA_HEADER_SIZE)) {
+		log_debug_metadata("Reading mda header sector from %s at %llu",
+			   	   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
 
-	if (!dev_read(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, mdah)) {
-		if (!dev_close(dev_area->dev))
-			stack;
-		return_0;
+		if (!dev_read(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, mdah)) {
+			if (!dev_close(dev_area->dev))
+				stack;
+			return_0;
+		}
+	} else {
+		log_debug_metadata("Copying mda header sector from %s buffer at %llu",
+			   	   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
+
+		memcpy(mdah, ld->buf + dev_area->start, MDA_HEADER_SIZE);
 	}
 
 	if (!dev_close(dev_area->dev))
@@ -369,7 +377,8 @@ static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev
 }
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area)
+				       struct device_area *dev_area,
+				       struct label_read_data *ld)
 {
 	struct mda_header *mdah;
 
@@ -378,7 +387,7 @@ struct mda_header *raw_read_mda_header(const struct format_type *fmt,
 		return NULL;
 	}
 
-	if (!_raw_read_mda_header(mdah, dev_area)) {
+	if (!_raw_read_mda_header(mdah, dev_area, ld)) {
 		dm_pool_free(fmt->cmd->mem, mdah);
 		return NULL;
 	}
@@ -491,7 +500,7 @@ static int _raw_holds_vgname(struct format_instance *fid,
 	if (!dev_open_readonly(dev_area->dev))
 		return_0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, dev_area)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, dev_area, NULL)))
 		return_0;
 
 	if (_find_vg_rlocn(dev_area, mdah, vgname, &noprecommit))
@@ -518,7 +527,7 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
 	char *desc;
 	uint32_t wrap = 0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, area)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, area, NULL)))
 		goto_out;
 
 	if (!(rlocn = _find_vg_rlocn(area, mdah, vgname, &precommitted))) {
@@ -633,7 +642,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 	if (!dev_open(mdac->area.dev))
 		return_0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, NULL)))
 		goto_out;
 
 	rlocn = _find_vg_rlocn(&mdac->area, mdah, old_vg_name ? : vg->name, &noprecommit);
@@ -739,7 +748,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
 	if (!found)
 		return 1;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, NULL)))
 		goto_out;
 
 	if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, old_vg_name ? : vg->name, &noprecommit))) {
@@ -849,7 +858,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
 	if (!dev_open(mdac->area.dev))
 		return_0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, NULL)))
 		goto_out;
 
 	if (!(rlocn = _find_vg_rlocn(&mdac->area, mdah, vg->name, &noprecommit))) {
@@ -1278,7 +1287,7 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu
 			continue;
 		}
 
-		if (!(mdah = raw_read_mda_header(fmt, &rl->dev_area))) {
+		if (!(mdah = raw_read_mda_header(fmt, &rl->dev_area, NULL))) {
 			stack;
 			goto close_dev;
 		}
@@ -1758,7 +1767,7 @@ static int _mda_export_text_raw(struct metadata_area *mda,
 	struct mda_context *mdc = (struct mda_context *) mda->metadata_locn;
 	char mdah[MDA_HEADER_SIZE]; /* temporary */
 
-	if (!mdc || !_raw_read_mda_header((struct mda_header *)mdah, &mdc->area))
+	if (!mdc || !_raw_read_mda_header((struct mda_header *)mdah, &mdc->area, NULL))
 		return 1; /* pretend the MDA does not exist */
 
 	return config_make_nodes(cft, parent, NULL,
diff --git a/lib/format_text/layout.h b/lib/format_text/layout.h
index 75a935ba5..a5ddecb46 100644
--- a/lib/format_text/layout.h
+++ b/lib/format_text/layout.h
@@ -81,7 +81,8 @@ struct mda_header {
 } __attribute__ ((packed));
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area);
+				       struct device_area *dev_area,
+				       struct label_read_data *ld);
 
 struct mda_lists {
 	struct dm_list dirs;
diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index ad323b6d2..b175f1b61 100644
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -311,6 +311,7 @@ static int _text_initialise_label(struct labeller *l __attribute__((unused)),
 struct _update_mda_baton {
 	struct lvmcache_info *info;
 	struct label *label;
+	struct label_read_data *ld;
 };
 
 static int _update_mda(struct metadata_area *mda, void *baton)
@@ -334,7 +335,7 @@ static int _update_mda(struct metadata_area *mda, void *baton)
 		return 1;
 	}
 
-	if (!(mdah = raw_read_mda_header(fmt, &mdac->area))) {
+	if (!(mdah = raw_read_mda_header(fmt, &mdac->area, p->ld))) {
 		stack;
 		goto close_dev;
 	}
@@ -350,6 +351,12 @@ static int _update_mda(struct metadata_area *mda, void *baton)
 		return 1;
 	}
 
+	/*
+	 * FIXME: vgname_from_mda reads metadata from mda location;
+	 * pass it ld so it can copy the metadata from ld->buf and
+	 * avoid reading the dev.
+	 */
+
 	if (vgname_from_mda(fmt, mdah, &mdac->area, &vgsummary,
 			     &mdac->free_sectors) &&
 	    !lvmcache_update_vgname_and_id(p->info, &vgsummary)) {
@@ -365,10 +372,18 @@ close_dev:
 	return 1;
 }
 
-static int _text_read(struct labeller *l, struct device *dev, void *buf,
-		 struct label **label)
+/*
+ * When label_read_data *ld is set, it means that we
+ * have asynchronously read the first ld->buf_len bytes
+ * of the device and already have that data, so we don't
+ * need do do any dev_read's (as long as the desired
+ * dev_read offset+size is less then ld->buf_len).
+ */
+
+static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
+		      struct label_read_data *ld, struct label **label)
 {
-	struct label_header *lh = (struct label_header *) buf;
+	struct label_header *lh = (struct label_header *) label_buf;
 	struct pv_header *pvhdr;
 	struct pv_header_extension *pvhdr_ext;
 	struct lvmcache_info *info;
@@ -380,7 +395,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *buf,
 	/*
 	 * PV header base
 	 */
-	pvhdr = (struct pv_header *) ((char *) buf + xlate32(lh->offset_xl));
+	pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl));
 
 	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
 				  FMT_TEXT_ORPHAN_VG_NAME,
@@ -436,6 +451,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *buf,
 out:
 	baton.info = info;
 	baton.label = *label;
+	baton.ld = ld;
 
 	if (!lvmcache_foreach_mda(info, _update_mda, &baton))
 		return_0;
diff --git a/lib/label/label.c b/lib/label/label.c
index 36af866b0..110ddf2a7 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -24,7 +24,6 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <libaio.h>
 
 /* FIXME Allow for larger labels?  Restricted to single sector currently */
 
@@ -335,7 +334,7 @@ int label_read(struct device *dev, struct label **labelp, uint64_t scan_sector)
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, dev, label_buf, labelp)) && *labelp) {
+	if ((r = (l->ops->read)(l, dev, label_buf, NULL, labelp)) && *labelp) {
 		(*labelp)->dev = dev;
 		(*labelp)->sector = sector;
 	} else {
@@ -448,18 +447,6 @@ struct label *label_create(struct labeller *labeller)
 	return label;
 }
 
-struct label_read_data {
-	char *buf; /* 2K aligned memory buffer */
-	struct iocb iocb;
-	struct device *dev;
-	struct dm_list list;
-	int buf_len; /* LABEL_SCAN_SIZE */
-	int try_sync;
-	int read_done;
-	int read_result;
-	int process_done;
-};
-
 /*
  * Start label aio read on a device.
  */
@@ -543,6 +530,9 @@ static int _label_read_async_process(struct cmd_context *cmd, struct label_read_
 	/*
 	 * Finds the sector from scanbuf containing the label and copies into label_buf.
 	 * label_buf: struct label_header + struct pv_header + struct pv_header_extension
+	 *
+	 * FIXME: we don't need to copy one sector from ld->buf into label_buf,
+	 * we can just point label_buf at one sector in ld->buf.
 	 */
 	if (!(l = _find_label_header(ld->dev, ld->buf, label_buf, &sector, 0))) {
 		/* Non-PVs exit here */
@@ -555,7 +545,7 @@ static int _label_read_async_process(struct cmd_context *cmd, struct label_read_
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, ld->dev, label_buf, &label)) && label) {
+	if ((r = (l->ops->read)(l, ld->dev, label_buf, ld, &label)) && label) {
 		label->dev = ld->dev;
 		label->sector = sector;
 	} else {
@@ -598,14 +588,23 @@ int label_scan_async(struct cmd_context *cmd)
 	dm_list_init(&label_read_list);
 
 	/*
-	 * "buf" is the buffer into which the first four sectors
-	 * of each device is read.
-	 * (LABEL_SCAN_SIZE is four 512-byte sectors, i.e. 2K).
+	 * "buf" is the buffer into which the first ASYNC_SCAN_SIZE bytes
+	 * of each device are read.  The memory for buf needs to be aligned.
+	 *
+	 * This data is meant to big large enough to cover all the
+	 * headers and metadata that need to be read from the device
+	 * during the label scan:
+	 *
+	 * 1. one of the first four sectors holds:
+	 *    label_header, pv_header, pv_header_extention
+	 *
+	 * 2. the mda_header whose location is found from 1,
+	 *    (is typically at 4096.)
 	 *
-	 * The label is expected to be one of the first four sectors.
-	 * buf needs to be aligned.
+	 * 3. the metadata whose location is from found 2,
+	 *    (is typically at 16896.)
 	 */
-	buf_len = LABEL_SCAN_SIZE;
+	buf_len = ASYNC_SCAN_SIZE;
 
 	/*
 	 * if aio setup fails, caller will revert to sync scan
@@ -686,7 +685,7 @@ int label_scan_async(struct cmd_context *cmd)
 		if (!_label_read_async_start(cmd, aio_ctx, ld))
 			ld->try_sync = 1;
 		else
-			log_debug_devs("Reading label sectors from device %s async", dev_name(ld->dev));
+			log_debug_devs("Reading sectors from device %s async", dev_name(ld->dev));
 	}
 
 	/*
@@ -694,7 +693,7 @@ int label_scan_async(struct cmd_context *cmd)
 	 */
 	dm_list_iterate_items(ld, &label_read_list) {
 		if (ld->try_sync) {
-			log_debug_devs("Reading label sectors from device %s async", dev_name(ld->dev));
+			log_debug_devs("Reading sectors from device %s trying sync", dev_name(ld->dev));
 
 			if (!dev_read(ld->dev, 0, ld->buf_len, ld->buf)) {
 				log_debug_devs("%s: Failed to read label area", dev_name(ld->dev));
@@ -815,7 +814,7 @@ static int _label_read_sync(struct cmd_context *cmd, struct device *dev)
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, dev, label_buf, &label)) && label) {
+	if ((r = (l->ops->read)(l, dev, label_buf, NULL, &label)) && label) {
 		label->dev = dev;
 		label->sector = sector;
 	} else {
diff --git a/lib/label/label.h b/lib/label/label.h
index 6a4a48dd5..798d457ba 100644
--- a/lib/label/label.h
+++ b/lib/label/label.h
@@ -20,6 +20,8 @@
 #include "device.h"
 #include "toolcontext.h"
 
+#include <libaio.h>
+
 #define LABEL_ID "LABELONE"
 #define LABEL_SIZE SECTOR_SIZE	/* Think very carefully before changing this */
 #define LABEL_SCAN_SECTORS 4L
@@ -29,6 +31,20 @@ struct labeller;
 
 void allow_reads_with_lvmetad(void);
 
+#define ASYNC_SCAN_SIZE (32 * 1024)
+
+struct label_read_data {
+	char *buf; /* ASYNC_SCAN_SIZE aligned memory buffer */
+	struct iocb iocb;
+	struct device *dev;
+	struct dm_list list;
+	int buf_len; /* ASYNC_SCAN_SIZE */
+	int try_sync;
+	int read_done;
+	int read_result;
+	int process_done;
+};
+
 /* On disk - 32 bytes */
 struct label_header {
 	int8_t id[8];		/* LABELONE */
@@ -64,7 +80,8 @@ struct label_ops {
 	 * Read a label from a volume.
 	 */
 	int (*read) (struct labeller * l, struct device * dev,
-		     void *buf, struct label ** label);
+		     void *label_buf,
+		     struct label_read_data *ld, struct label ** label);
 
 	/*
 	 * Additional consistency checks for the paranoid.