Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 11 Jun 2024 00:42:05 GMT
From:      Ryan Libby <rlibby@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: a2fda816eb05 - main - virstor: write large maps in chunks during label
Message-ID:  <202406110042.45B0g5b2082652@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by rlibby:

URL: https://cgit.FreeBSD.org/src/commit/?id=a2fda816eb054d5873be223ef2461741dfcc253c

commit a2fda816eb054d5873be223ef2461741dfcc253c
Author:     Ryan Libby <rlibby@FreeBSD.org>
AuthorDate: 2024-06-11 00:36:20 +0000
Commit:     Ryan Libby <rlibby@FreeBSD.org>
CommitDate: 2024-06-11 00:36:20 +0000

    virstor: write large maps in chunks during label
    
    During the initial label of a virstor device, write out the allocation
    map in chunks if it is large (> 1 MB) in order to avoid large mallocs.
    
    Even though the kernel virstor geom may still do a large malloc to
    represent the allocation map, this may still be useful to avoid a
    ulimit.
    
    Reviewed by:    markj
    Sponsored by:   Dell EMC Isilon
    Differential Revision:  https://reviews.freebsd.org/D45517
---
 lib/geom/virstor/geom_virstor.c | 59 ++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/lib/geom/virstor/geom_virstor.c b/lib/geom/virstor/geom_virstor.c
index 131ece0107c7..6a7dfb27fe43 100644
--- a/lib/geom/virstor/geom_virstor.c
+++ b/lib/geom/virstor/geom_virstor.c
@@ -157,8 +157,7 @@ virstor_label(struct gctl_req *req)
 	char param[32];
 	int hardcode, nargs, error;
 	struct virstor_map_entry *map;
-	size_t total_chunks;	/* We'll run out of memory if
-				   this needs to be bigger. */
+	size_t total_chunks, write_max_map_entries;
 	unsigned int map_chunks; /* Chunks needed by the map (map size). */
 	size_t map_size;	/* In bytes. */
 	ssize_t written;
@@ -325,28 +324,56 @@ virstor_label(struct gctl_req *req)
 		sprintf(param, "%s%s", _PATH_DEV, name);
 		fd = open(param, O_RDWR);
 	}
-	if (fd < 0)
+	if (fd < 0) {
 		gctl_error(req, "Cannot open provider %s to write map", name);
+		return;
+	}
 
-	/* Do it with calloc because there might be a need to set up chunk flags
-	 * in the future */
-	map = calloc(total_chunks, sizeof(*map));
+	/*
+	 * Initialize and write the map.  Don't malloc the whole map at once,
+	 * in case it's large.  Use calloc because there might be a need to set
+	 * up chunk flags in the future.
+	 */
+	write_max_map_entries = 1024 * 1024 / sizeof(*map);
+	if (write_max_map_entries > total_chunks)
+		write_max_map_entries = total_chunks;
+	map = calloc(write_max_map_entries, sizeof(*map));
 	if (map == NULL) {
 		gctl_error(req,
 		    "Out of memory (need %zu bytes for allocation map)",
-		    map_size);
+		    write_max_map_entries * sizeof(*map));
+		close(fd);
+		return;
 	}
-
-	written = pwrite(fd, map, map_size, 0);
-	free(map);
-	if ((size_t)written != map_size) {
-		if (verbose) {
-			fprintf(stderr, "\nTried to write %zu, written %zd (%s)\n",
-			    map_size, written, strerror(errno));
+	for (size_t chunk = 0; chunk < total_chunks;
+	    chunk += write_max_map_entries) {
+		size_t bytes_to_write, entries_to_write;
+
+		entries_to_write = total_chunks - chunk;
+		if (entries_to_write > write_max_map_entries)
+			entries_to_write = write_max_map_entries;
+		bytes_to_write = entries_to_write * sizeof(*map);
+		for (size_t off = 0; off < bytes_to_write; off += written) {
+			written = write(fd, ((char *)map) + off,
+			    bytes_to_write - off);
+			if (written < 0) {
+				if (verbose) {
+					fprintf(stderr,
+					    "\nError writing map at offset "
+					    "%zu of %zu: %s\n",
+					    chunk * sizeof(*map) + off,
+					    map_size, strerror(errno));
+				}
+				gctl_error(req,
+				    "Error writing out allocation map!");
+				free(map);
+				close(fd);
+				return;
+			}
 		}
-		gctl_error(req, "Error writing out allocation map!");
-		return;
 	}
+	free(map);
+	map = NULL;
 	close (fd);
 
 	if (verbose)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202406110042.45B0g5b2082652>