linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Various cleanups and minor fixes
@ 2012-03-20 16:54 Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 01/11] super1.c don't keep recalculating bitmap pointer Jes.Sorensen
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

This patch set implements a number of smaller cleanups, as well as
fixing a couple of cases where we don't zero out the full buffer we
just obtained via malloc.

The most controversial is probably patch 06 which caches the device
block size for aligned read/writes in super1.c to avoid calling the
ioctl() on each write.

I also introduced some generalized ROUND_UP macros to avoid the manual
pointer and size manipulation in various places.

The ddf code I am unable to test as I don't have any raids with ddf
layout, but it does compile.

Cheers,
Jes


Jes Sorensen (11):
  super1.c don't keep recalculating bitmap pointer
  Define and use SUPER1_SIZE for allocations
  init_super1() memset full buffer allocated for superblock
  match_metadata_desc1(): Use calloc instead of malloc+memset
  Use 4K buffer alignment for superblock allocations
  Use struct align_fd to cache fd's block size for aligned reads/writes
  match_metadata_desc0(): Use calloc instead of malloc+memset
  Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR()
  super1.c: use ROUND_UP/ROUND_UP_PTR
  super-intel.c: Use ROUND_UP() instead of manually coding it
  __write_init_super_ddf(): Use posix_memalign() instead of static
    aligned buffer

 mdadm.h       |    8 ++--
 super-ddf.c   |   19 ++++++----
 super-intel.c |    2 +-
 super0.c      |    6 ++--
 super1.c      |  111 +++++++++++++++++++++++++++++++++++----------------------
 5 files changed, 87 insertions(+), 59 deletions(-)

-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11] super1.c don't keep recalculating bitmap pointer
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 02/11] Define and use SUPER1_SIZE for allocations Jes.Sorensen
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

We just calculated the pointer to the bitmap, so use it instead of
recalculating.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/super1.c b/super1.c
index 341ad53..4e3bf7b 100644
--- a/super1.c
+++ b/super1.c
@@ -1389,8 +1389,7 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 	 * should get that written out.
 	 */
 	locate_bitmap1(st, fd);
-	if (aread(fd, ((char*)super)+1024, 512)
-	    != 512)
+	if (aread(fd, bsb, 512) != 512)
 		goto no_bitmap;
 
 	uuid_from_super1(st, uuid);
@@ -1657,7 +1656,7 @@ static int write_bitmap1(struct supertype *st, int fd)
 		return -ENOMEM;
 
 	memset(buf, 0xff, 4096);
-	memcpy(buf, ((char*)sb)+1024, sizeof(bitmap_super_t));
+	memcpy(buf, (char *)bms, sizeof(bitmap_super_t));
 
 	towrite = __le64_to_cpu(bms->sync_size) / (__le32_to_cpu(bms->chunksize)>>9);
 	towrite = (towrite+7) >> 3; /* bits to bytes */
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/11] Define and use SUPER1_SIZE for allocations
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 01/11] super1.c don't keep recalculating bitmap pointer Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 03/11] init_super1() memset full buffer allocated for superblock Jes.Sorensen
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Use a #define rather than calculate the size of the superblock buffer
on every allocation.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/super1.c b/super1.c
index 4e3bf7b..4c75f21 100644
--- a/super1.c
+++ b/super1.c
@@ -102,6 +102,8 @@ struct misc_dev_info {
 
 #define	MD_FEATURE_ALL			(1|2|4)
 
+#define SUPER1_SIZE	(1024 + 512 + sizeof(struct misc_dev_info))
+
 #ifndef offsetof
 #define offsetof(t,f) ((size_t)&(((t*)0)->f))
 #endif
@@ -835,8 +837,7 @@ static int init_super1(struct supertype *st, mdu_array_info_t *info,
 	int rfd;
 	char defname[10];
 
-	if (posix_memalign((void**)&sb, 512, (1024 + 512 + 
-			   sizeof(struct misc_dev_info))) != 0) {
+	if (posix_memalign((void**)&sb, 512, SUPER1_SIZE) != 0) {
 		fprintf(stderr, Name
 			": %s could not allocate superblock\n", __func__);
 		return 0;
@@ -1221,15 +1222,12 @@ static int compare_super1(struct supertype *st, struct supertype *tst)
 		return 1;
 
 	if (!first) {
-		if (posix_memalign((void**)&first, 512,
-			       1024 + 512 +
-			       sizeof(struct misc_dev_info)) != 0) {
+		if (posix_memalign((void**)&first, 512, SUPER1_SIZE) != 0) {
 			fprintf(stderr, Name
 				": %s could not allocate superblock\n", __func__);
 			return 1;
 		}
-		memcpy(first, second, 1024 + 512 + 
-		       sizeof(struct misc_dev_info));
+		memcpy(first, second, SUPER1_SIZE);
 		st->sb = first;
 		return 0;
 	}
@@ -1336,9 +1334,7 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 		return 1;
 	}
 
-	if (posix_memalign((void**)&super, 512,
-		       1024 + 512 +
-		       sizeof(struct misc_dev_info)) != 0) {
+	if (posix_memalign((void**)&super, 512, SUPER1_SIZE) != 0) {
 		fprintf(stderr, Name ": %s could not allocate superblock\n",
 			__func__);
 		return 1;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/11] init_super1() memset full buffer allocated for superblock
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 01/11] super1.c don't keep recalculating bitmap pointer Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 02/11] Define and use SUPER1_SIZE for allocations Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 04/11] match_metadata_desc1(): Use calloc instead of malloc+memset Jes.Sorensen
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Avoid possibly using stale data in bitmap and misc area of superblock.
In addition, remove superfluous memsets already covered by memset of
full superblock.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/super1.c b/super1.c
index 4c75f21..1f0c8bc 100644
--- a/super1.c
+++ b/super1.c
@@ -842,7 +842,7 @@ static int init_super1(struct supertype *st, mdu_array_info_t *info,
 			": %s could not allocate superblock\n", __func__);
 		return 0;
 	}
-	memset(sb, 0, 1024);
+	memset(sb, 0, SUPER1_SIZE);
 
 	st->sb = sb;
 	if (info == NULL) {
@@ -877,7 +877,6 @@ static int init_super1(struct supertype *st, mdu_array_info_t *info,
 		sprintf(defname, "%d", info->md_minor);
 		name = defname;
 	}
-	memset(sb->set_name, 0, 32);
 	if (homehost &&
 	    strchr(name, ':')== NULL &&
 	    strlen(homehost)+1+strlen(name) < 32) {
@@ -907,7 +906,6 @@ static int init_super1(struct supertype *st, mdu_array_info_t *info,
 		sb->resync_offset = 0;
 	sb->max_dev = __cpu_to_le32((1024- sizeof(struct mdp_superblock_1))/
 				    sizeof(sb->dev_roles[0]));
-	memset(sb->pad3, 0, sizeof(sb->pad3));
 
 	memset(sb->dev_roles, 0xff, 1024 - sizeof(struct mdp_superblock_1));
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/11] match_metadata_desc1(): Use calloc instead of malloc+memset
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (2 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 03/11] init_super1() memset full buffer allocated for superblock Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 05/11] Use 4K buffer alignment for superblock allocations Jes.Sorensen
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/super1.c b/super1.c
index 1f0c8bc..ca9e4e4 100644
--- a/super1.c
+++ b/super1.c
@@ -1401,10 +1401,10 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 
 static struct supertype *match_metadata_desc1(char *arg)
 {
-	struct supertype *st = malloc(sizeof(*st));
-	if (!st) return st;
+	struct supertype *st = calloc(1, sizeof(*st));
+	if (!st)
+		return st;
 
-	memset(st, 0, sizeof(*st));
 	st->container_dev = NoMdDev;
 	st->ss = &super1;
 	st->max_devs = 384;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/11] Use 4K buffer alignment for superblock allocations
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (3 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 04/11] match_metadata_desc1(): Use calloc instead of malloc+memset Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 06/11] Use struct align_fd to cache fd's block size for aligned reads/writes Jes.Sorensen
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

To better accommodate 4K sector drives, use 4K buffer alignment for
superblock buffers.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/super1.c b/super1.c
index ca9e4e4..513f406 100644
--- a/super1.c
+++ b/super1.c
@@ -837,7 +837,7 @@ static int init_super1(struct supertype *st, mdu_array_info_t *info,
 	int rfd;
 	char defname[10];
 
-	if (posix_memalign((void**)&sb, 512, SUPER1_SIZE) != 0) {
+	if (posix_memalign((void**)&sb, 4096, SUPER1_SIZE) != 0) {
 		fprintf(stderr, Name
 			": %s could not allocate superblock\n", __func__);
 		return 0;
@@ -1220,7 +1220,7 @@ static int compare_super1(struct supertype *st, struct supertype *tst)
 		return 1;
 
 	if (!first) {
-		if (posix_memalign((void**)&first, 512, SUPER1_SIZE) != 0) {
+		if (posix_memalign((void**)&first, 4096, SUPER1_SIZE) != 0) {
 			fprintf(stderr, Name
 				": %s could not allocate superblock\n", __func__);
 			return 1;
@@ -1332,7 +1332,7 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 		return 1;
 	}
 
-	if (posix_memalign((void**)&super, 512, SUPER1_SIZE) != 0) {
+	if (posix_memalign((void**)&super, 4096, SUPER1_SIZE) != 0) {
 		fprintf(stderr, Name ": %s could not allocate superblock\n",
 			__func__);
 		return 1;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/11] Use struct align_fd to cache fd's block size for aligned reads/writes
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (4 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 05/11] Use 4K buffer alignment for superblock allocations Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 07/11] match_metadata_desc0(): Use calloc instead of malloc+memset Jes.Sorensen
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This uses a struct to cache the block size for aligned reads/writes,
to avoid repeated ioctl(BLKSSZGET) calls.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |   75 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/super1.c b/super1.c
index 513f406..350e27a 100644
--- a/super1.c
+++ b/super1.c
@@ -137,8 +137,25 @@ static unsigned int calc_sb_1_csum(struct mdp_superblock_1 * sb)
 	return __cpu_to_le32(csum);
 }
 
+/*
+ * Information related to file descriptor used for aligned reads/writes.
+ * Cache the block size.
+ */
+struct align_fd {
+	int fd;
+	int blk_sz;
+};
+
+static void init_afd(struct align_fd *afd, int fd)
+{
+	afd->fd = fd;
+
+	if (ioctl(afd->fd, BLKSSZGET, &afd->blk_sz) != 0)
+		afd->blk_sz = 512;
+}
+
 static char abuf[4096+4096];
-static int aread(int fd, void *buf, int len)
+static int aread(struct align_fd *afd, void *buf, int len)
 {
 	/* aligned read.
 	 * On devices with a 4K sector size, we need to read
@@ -148,26 +165,30 @@ static int aread(int fd, void *buf, int len)
 	int bsize, iosize;
 	char *b;
 	int n;
-	if (ioctl(fd, BLKSSZGET, &bsize) != 0)
-		bsize = 512;
 
-	if (bsize > 4096 || len > 4096)
+	bsize = afd->blk_sz;
+
+	if (!bsize || bsize > 4096 || len > 4096) {
+		if (!bsize)
+			fprintf(stderr, "WARNING - aread() called with "
+				"invalid block size\n");
 		return -1;
+	}
 	b = (char*)(((long)(abuf+4096))&~4095UL);
 
 	for (iosize = 0; iosize < len; iosize += bsize)
 		;
-	n = read(fd, b, iosize);
+	n = read(afd->fd, b, iosize);
 	if (n <= 0)
 		return n;
-	lseek(fd, len - n, 1);
+	lseek(afd->fd, len - n, 1);
 	if (n > len)
 		n = len;
 	memcpy(buf, b, n);
 	return n;
 }
 
-static int awrite(int fd, void *buf, int len)
+static int awrite(struct align_fd *afd, void *buf, int len)
 {
 	/* aligned write.
 	 * On devices with a 4K sector size, we need to write
@@ -178,27 +199,31 @@ static int awrite(int fd, void *buf, int len)
 	int bsize, iosize;
 	char *b;
 	int n;
-	if (ioctl(fd, BLKSSZGET, &bsize) != 0)
-		bsize = 512;
-	if (bsize > 4096 || len > 4096)
+
+	bsize = afd->blk_sz;
+	if (!bsize || bsize > 4096 || len > 4096) {
+		if (!bsize)
+			fprintf(stderr, "WARNING - awrite() called with "
+				"invalid block size\n");
 		return -1;
+	}
 	b = (char*)(((long)(abuf+4096))&~4095UL);
 
 	for (iosize = 0; iosize < len ; iosize += bsize)
 		;
 
 	if (len != iosize) {
-		n = read(fd, b, iosize);
+		n = read(afd->fd, b, iosize);
 		if (n <= 0)
 			return n;
-		lseek(fd, -n, 1);
+		lseek(afd->fd, -n, 1);
 	}
 
 	memcpy(b, buf, len);
-	n = write(fd, b, iosize);
+	n = write(afd->fd, b, iosize);
 	if (n <= 0)
 		return n;
-	lseek(fd, len - n, 1);
+	lseek(afd->fd, len - n, 1);
 	return len;
 }
 
@@ -962,6 +987,7 @@ static int store_super1(struct supertype *st, int fd)
 {
 	struct mdp_superblock_1 *sb = st->sb;
 	unsigned long long sb_offset;
+	struct align_fd afd;
 	int sbsize;
 	unsigned long long dsize;
 
@@ -973,6 +999,8 @@ static int store_super1(struct supertype *st, int fd)
 	if (dsize < 24)
 		return 2;
 
+	init_afd(&afd, fd);
+
 	/*
 	 * Calculate the position of the superblock.
 	 * It is always aligned to a 4K boundary and
@@ -1012,7 +1040,7 @@ static int store_super1(struct supertype *st, int fd)
 	sbsize = sizeof(*sb) + 2 * __le32_to_cpu(sb->max_dev);
 	sbsize = (sbsize+511)&(~511UL);
 
-	if (awrite(fd, sb, sbsize) != sbsize)
+	if (awrite(&afd, sb, sbsize) != sbsize)
 		return 4;
 
 	if (sb->feature_map & __cpu_to_le32(MD_FEATURE_BITMAP_OFFSET)) {
@@ -1020,9 +1048,8 @@ static int store_super1(struct supertype *st, int fd)
 			(((char*)sb)+1024);
 		if (__le32_to_cpu(bm->magic) == BITMAP_MAGIC) {
 			locate_bitmap1(st, fd);
-			if (awrite(fd, bm, sizeof(*bm)) !=
-			    sizeof(*bm))
-			    return 5;
+			if (awrite(&afd, bm, sizeof(*bm)) != sizeof(*bm))
+				return 5;
 		}
 	}
 	fsync(fd);
@@ -1250,9 +1277,12 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 	int uuid[4];
 	struct bitmap_super_s *bsb;
 	struct misc_dev_info *misc;
+	struct align_fd afd;
 
 	free_super1(st);
 
+	init_afd(&afd, fd);
+
 	if (st->ss == NULL || st->minor_version == -1) {
 		int bestvers = -1;
 		struct supertype tst;
@@ -1338,7 +1368,7 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 		return 1;
 	}
 
-	if (aread(fd, super, 1024) != 1024) {
+	if (aread(&afd, super, 1024) != 1024) {
 		if (devname)
 			fprintf(stderr, Name ": Cannot read superblock on %s\n",
 				devname);
@@ -1383,7 +1413,7 @@ static int load_super1(struct supertype *st, int fd, char *devname)
 	 * should get that written out.
 	 */
 	locate_bitmap1(st, fd);
-	if (aread(fd, bsb, 512) != 512)
+	if (aread(&afd, bsb, 512) != 512)
 		goto no_bitmap;
 
 	uuid_from_super1(st, uuid);
@@ -1643,6 +1673,9 @@ static int write_bitmap1(struct supertype *st, int fd)
 	int rv = 0;
 	void *buf;
 	int towrite, n;
+	struct align_fd afd;
+
+	init_afd(&afd, fd);
 
 	locate_bitmap1(st, fd);
 
@@ -1660,7 +1693,7 @@ static int write_bitmap1(struct supertype *st, int fd)
 		n = towrite;
 		if (n > 4096)
 			n = 4096;
-		n = awrite(fd, buf, n);
+		n = awrite(&afd, buf, n);
 		if (n > 0)
 			towrite -= n;
 		else
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/11] match_metadata_desc0(): Use calloc instead of malloc+memset
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (5 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 06/11] Use struct align_fd to cache fd's block size for aligned reads/writes Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 08/11] Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR() Jes.Sorensen
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super0.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/super0.c b/super0.c
index d53f025..eca40d2 100644
--- a/super0.c
+++ b/super0.c
@@ -942,10 +942,10 @@ static int load_super0(struct supertype *st, int fd, char *devname)
 
 static struct supertype *match_metadata_desc0(char *arg)
 {
-	struct supertype *st = malloc(sizeof(*st));
-	if (!st) return st;
+	struct supertype *st = calloc(1, sizeof(*st));
+	if (!st)
+		return st;
 
-	memset(st, 0, sizeof(*st));
 	st->container_dev = NoMdDev;
 	st->ss = &super0;
 	st->info = NULL;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/11] Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR()
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (6 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 07/11] match_metadata_desc0(): Use calloc instead of malloc+memset Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 09/11] super1.c: use ROUND_UP/ROUND_UP_PTR Jes.Sorensen
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 mdadm.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mdadm.h b/mdadm.h
index 45198bb..314f3ac 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1252,10 +1252,10 @@ static inline int dev2minor(int d)
 	return (-1-d) << MdpMinorShift;
 }
 
-static inline int ROUND_UP(int a, int base)
-{
-	return ((a+base-1)/base)*base;
-}
+#define _ROUND_UP(val, base)	(((val) + (base)) & ~(base - 1))
+#define ROUND_UP(val, base)	_ROUND_UP(val, (typeof(val))(base))
+#define ROUND_UP_PTR(ptr, base)	((typeof(ptr)) \
+				 (ROUND_UP((unsigned long)(ptr), base)))
 
 static inline int is_subarray(char *vers)
 {
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/11] super1.c: use ROUND_UP/ROUND_UP_PTR
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (7 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 08/11] Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR() Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 10/11] super-intel.c: Use ROUND_UP() instead of manually coding it Jes.Sorensen
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super1.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/super1.c b/super1.c
index 350e27a..612ae98 100644
--- a/super1.c
+++ b/super1.c
@@ -174,7 +174,7 @@ static int aread(struct align_fd *afd, void *buf, int len)
 				"invalid block size\n");
 		return -1;
 	}
-	b = (char*)(((long)(abuf+4096))&~4095UL);
+	b = ROUND_UP_PTR((char *)abuf, 4096);
 
 	for (iosize = 0; iosize < len; iosize += bsize)
 		;
@@ -207,7 +207,7 @@ static int awrite(struct align_fd *afd, void *buf, int len)
 				"invalid block size\n");
 		return -1;
 	}
-	b = (char*)(((long)(abuf+4096))&~4095UL);
+	b = ROUND_UP_PTR((char *)abuf, 4096);
 
 	for (iosize = 0; iosize < len ; iosize += bsize)
 		;
@@ -1037,8 +1037,7 @@ static int store_super1(struct supertype *st, int fd)
 	if (lseek64(fd, sb_offset << 9, 0)< 0LL)
 		return 3;
 
-	sbsize = sizeof(*sb) + 2 * __le32_to_cpu(sb->max_dev);
-	sbsize = (sbsize+511)&(~511UL);
+	sbsize = ROUND_UP(sizeof(*sb) + 2 * __le32_to_cpu(sb->max_dev), 512);
 
 	if (awrite(&afd, sb, sbsize) != sbsize)
 		return 4;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/11] super-intel.c: Use ROUND_UP() instead of manually coding it
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (8 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 09/11] super1.c: use ROUND_UP/ROUND_UP_PTR Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 16:54 ` [PATCH 11/11] __write_init_super_ddf(): Use posix_memalign() instead of static aligned buffer Jes.Sorensen
  2012-03-20 21:11 ` [PATCH 00/11] Various cleanups and minor fixes NeilBrown
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super-intel.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 958edb5..a351b60 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -318,7 +318,7 @@ static void set_migr_type(struct imsm_dev *dev, __u8 migr_type)
 
 static unsigned int sector_count(__u32 bytes)
 {
-	return ((bytes + (512-1)) & (~(512-1))) / 512;
+	return ROUND_UP(bytes, 512) / 512;
 }
 
 static unsigned int mpb_sectors(struct imsm_super *mpb)
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/11] __write_init_super_ddf(): Use posix_memalign() instead of static aligned buffer
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (9 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 10/11] super-intel.c: Use ROUND_UP() instead of manually coding it Jes.Sorensen
@ 2012-03-20 16:54 ` Jes.Sorensen
  2012-03-20 21:11 ` [PATCH 00/11] Various cleanups and minor fixes NeilBrown
  11 siblings, 0 replies; 14+ messages in thread
From: Jes.Sorensen @ 2012-03-20 16:54 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 super-ddf.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/super-ddf.c b/super-ddf.c
index abd6793..5e51c4c 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -2326,8 +2326,7 @@ static int remove_from_super_ddf(struct supertype *st, mdu_disk_info_t *dk)
  * called when creating a container or adding another device to a
  * container.
  */
-
-static unsigned char null_conf[4096+512];
+#define NULL_CONF_SZ	4096
 
 static int __write_init_super_ddf(struct supertype *st)
 {
@@ -2340,6 +2339,12 @@ static int __write_init_super_ddf(struct supertype *st)
 	int attempts = 0;
 	int successes = 0;
 	unsigned long long size, sector;
+	char *null_aligned;
+
+	if (posix_memalign((void**)&null_aligned, 4096, NULL_CONF_SZ) != 0) {
+		return -ENOMEM;
+	}
+	memset(null_aligned, 0xff, NULL_CONF_SZ);
 
 	/* try to write updated metadata,
 	 * if we catch a failure move on to the next disk
@@ -2409,14 +2414,11 @@ static int __write_init_super_ddf(struct supertype *st)
 				if (write(fd, &c->conf, conf_size) < 0)
 					break;
 			} else {
-				char *null_aligned = (char*)((((unsigned long)null_conf)+511)&~511UL);
-				if (null_conf[0] != 0xff)
-					memset(null_conf, 0xff, sizeof(null_conf));
 				unsigned int togo = conf_size;
-				while (togo > sizeof(null_conf)-512) {
-					if (write(fd, null_aligned, sizeof(null_conf)-512) < 0)
+				while (togo > NULL_CONF_SZ) {
+					if (write(fd, null_aligned, NULL_CONF_SZ) < 0)
 						break;
-					togo -= sizeof(null_conf)-512;
+					togo -= NULL_CONF_SZ;
 				}
 				if (write(fd, null_aligned, togo) < 0)
 					break;
@@ -2435,6 +2437,7 @@ static int __write_init_super_ddf(struct supertype *st)
 			continue;
 		successes++;
 	}
+	free(null_aligned);
 
 	return attempts != successes;
 }
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/11] Various cleanups and minor fixes
  2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
                   ` (10 preceding siblings ...)
  2012-03-20 16:54 ` [PATCH 11/11] __write_init_super_ddf(): Use posix_memalign() instead of static aligned buffer Jes.Sorensen
@ 2012-03-20 21:11 ` NeilBrown
  2012-03-21  7:58   ` Jes Sorensen
  11 siblings, 1 reply; 14+ messages in thread
From: NeilBrown @ 2012-03-20 21:11 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: linux-raid

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

On Tue, 20 Mar 2012 17:54:05 +0100 Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> This patch set implements a number of smaller cleanups, as well as
> fixing a couple of cases where we don't zero out the full buffer we
> just obtained via malloc.

All applied and push out - thanks.  There were a few conflict which some
stuff I was working on, but nothing serious.

> 
> The most controversial is probably patch 06 which caches the device
> block size for aligned read/writes in super1.c to avoid calling the
> ioctl() on each write.

Yeah, it's a bit ugly, but seems reasonable.

> 
> I also introduced some generalized ROUND_UP macros to avoid the manual
> pointer and size manipulation in various places.
> 
> The ddf code I am unable to test as I don't have any raids with ddf
> layout, but it does compile.
> 
> Cheers,
> Jes

I think it might be nearly time for 3.2.4.  Do you have any thoughts about
that?

Thanks,
NeilBrown


> 
> 
> Jes Sorensen (11):
>   super1.c don't keep recalculating bitmap pointer
>   Define and use SUPER1_SIZE for allocations
>   init_super1() memset full buffer allocated for superblock
>   match_metadata_desc1(): Use calloc instead of malloc+memset
>   Use 4K buffer alignment for superblock allocations
>   Use struct align_fd to cache fd's block size for aligned reads/writes
>   match_metadata_desc0(): Use calloc instead of malloc+memset
>   Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR()
>   super1.c: use ROUND_UP/ROUND_UP_PTR
>   super-intel.c: Use ROUND_UP() instead of manually coding it
>   __write_init_super_ddf(): Use posix_memalign() instead of static
>     aligned buffer
> 
>  mdadm.h       |    8 ++--
>  super-ddf.c   |   19 ++++++----
>  super-intel.c |    2 +-
>  super0.c      |    6 ++--
>  super1.c      |  111 +++++++++++++++++++++++++++++++++++----------------------
>  5 files changed, 87 insertions(+), 59 deletions(-)
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/11] Various cleanups and minor fixes
  2012-03-20 21:11 ` [PATCH 00/11] Various cleanups and minor fixes NeilBrown
@ 2012-03-21  7:58   ` Jes Sorensen
  0 siblings, 0 replies; 14+ messages in thread
From: Jes Sorensen @ 2012-03-21  7:58 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid

On 03/20/12 22:11, NeilBrown wrote:
> On Tue, 20 Mar 2012 17:54:05 +0100 Jes.Sorensen@redhat.com wrote:
> 
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> This patch set implements a number of smaller cleanups, as well as
>> fixing a couple of cases where we don't zero out the full buffer we
>> just obtained via malloc.
> 
> All applied and push out - thanks.  There were a few conflict which some
> stuff I was working on, but nothing serious.

Excellent, thanks!

>> The most controversial is probably patch 06 which caches the device
>> block size for aligned read/writes in super1.c to avoid calling the
>> ioctl() on each write.
> 
> Yeah, it's a bit ugly, but seems reasonable.

Yeah, I was trying to find the cleanest way of doing it. I am
contemplating whether it makes sense to try and push this up the stack,
but I figured starting out small like this would be better.

> I think it might be nearly time for 3.2.4.  Do you have any thoughts about
> that?

I think it would be a good time - I have found it relatively quiet on
the bug report front since the 3.2.3 update, so jumping to 3.2.4 sounds
good.

Cheers,
Jes

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-03-21  7:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-20 16:54 [PATCH 00/11] Various cleanups and minor fixes Jes.Sorensen
2012-03-20 16:54 ` [PATCH 01/11] super1.c don't keep recalculating bitmap pointer Jes.Sorensen
2012-03-20 16:54 ` [PATCH 02/11] Define and use SUPER1_SIZE for allocations Jes.Sorensen
2012-03-20 16:54 ` [PATCH 03/11] init_super1() memset full buffer allocated for superblock Jes.Sorensen
2012-03-20 16:54 ` [PATCH 04/11] match_metadata_desc1(): Use calloc instead of malloc+memset Jes.Sorensen
2012-03-20 16:54 ` [PATCH 05/11] Use 4K buffer alignment for superblock allocations Jes.Sorensen
2012-03-20 16:54 ` [PATCH 06/11] Use struct align_fd to cache fd's block size for aligned reads/writes Jes.Sorensen
2012-03-20 16:54 ` [PATCH 07/11] match_metadata_desc0(): Use calloc instead of malloc+memset Jes.Sorensen
2012-03-20 16:54 ` [PATCH 08/11] Generalize ROUND_UP() macro and introduce matching ROUND_UP_PTR() Jes.Sorensen
2012-03-20 16:54 ` [PATCH 09/11] super1.c: use ROUND_UP/ROUND_UP_PTR Jes.Sorensen
2012-03-20 16:54 ` [PATCH 10/11] super-intel.c: Use ROUND_UP() instead of manually coding it Jes.Sorensen
2012-03-20 16:54 ` [PATCH 11/11] __write_init_super_ddf(): Use posix_memalign() instead of static aligned buffer Jes.Sorensen
2012-03-20 21:11 ` [PATCH 00/11] Various cleanups and minor fixes NeilBrown
2012-03-21  7:58   ` Jes Sorensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).