From c33eebe61ad00c75dece78c6e1bb07fd1aba444e Mon Sep 17 00:00:00 2001 From: Markus Metz <33666869+metzm@users.noreply.github.com> Date: Sun, 3 Sep 2023 11:34:17 +0200 Subject: [PATCH] parallelized raster modules: fix buffer sizes (#3070) * parallelized raster modules: fix buffer sizes * safety check for unsigned integer subtraction --- raster/r.neighbors/main.c | 32 ++++++++++++++++++++++---------- raster/r.patch/main.c | 16 +++++++++++++--- raster/r.resamp.filter/main.c | 20 +++++++++++++++++--- raster/r.series/main.c | 31 +++++++++++++++++++++---------- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/raster/r.neighbors/main.c b/raster/r.neighbors/main.c index fbc3584235d..a515e7505f5 100644 --- a/raster/r.neighbors/main.c +++ b/raster/r.neighbors/main.c @@ -148,7 +148,7 @@ int main(int argc, char *argv[]) int *readrow; int nrows, ncols, brows; int i, n, t; - size_t size; + size_t size, in_buf_size, out_buf_size; struct Colors colr; struct Cell_head cellhd; struct Cell_head window; @@ -338,15 +338,6 @@ int main(int argc, char *argv[]) nrows = Rast_window_rows(); ncols = Rast_window_cols(); - brows = atoi(parm.memory->answer) * ((1 << 20) / sizeof(DCELL)) / ncols; - /* set the output buffer rows to be at most covering the entire map */ - if (brows > nrows) { - brows = nrows; - } - /* but at least the number of threads */ - if (brows < ncb.threads) { - brows = ncb.threads; - } /* open raster maps */ in_fd = G_malloc(sizeof(int) * ncb.threads); @@ -368,6 +359,27 @@ int main(int argc, char *argv[]) outputs = G_calloc(num_outputs, sizeof(struct output)); + /* memory reserved for input */ + in_buf_size = (Rast_window_cols() + 2 * ncb.dist) * sizeof(DCELL) * + ncb.nsize * ncb.threads; + /* memory available for output buffer */ + out_buf_size = (size_t)atoi(parm.memory->answer) * (1 << 20); + /* size_t is unsigned, check if any memory is left for output buffer */ + if (out_buf_size <= in_buf_size) + out_buf_size = 0; + else + out_buf_size -= in_buf_size; + /* number of buffered rows for all output maps */ + brows = out_buf_size / (sizeof(DCELL) * ncols * num_outputs); + /* set the output buffer rows to be at most covering the entire map */ + if (brows > nrows) { + brows = nrows; + } + /* but at least the number of threads */ + if (brows < ncb.threads) { + brows = ncb.threads; + } + /* read the weights */ weights = 0; ncb.weights = NULL; diff --git a/raster/r.patch/main.c b/raster/r.patch/main.c index 7ce24f8163a..3835e287440 100644 --- a/raster/r.patch/main.c +++ b/raster/r.patch/main.c @@ -41,7 +41,7 @@ int main(int argc, char *argv[]) int colr_ok; int outfd; RASTER_MAP_TYPE out_type, map_type; - size_t out_cell_size; + size_t out_cell_size, in_buf_size, out_buf_size; struct History history; void **presult, **patch; void *outbuf; @@ -135,7 +135,7 @@ int main(int argc, char *argv[]) for (t = 0; t < nprocs; t++) infd[t] = G_malloc(nfiles * sizeof(int)); thread_statf = G_malloc(nprocs * (nfiles * sizeof(struct Cell_stats))); - for (int t = 0; t < nprocs; t++) { + for (t = 0; t < nprocs; t++) { thread_statf[t] = G_malloc(nfiles * sizeof(struct Cell_stats)); } cellhd = G_malloc(nfiles * sizeof(struct Cell_head)); @@ -180,7 +180,17 @@ int main(int argc, char *argv[]) nrows = Rast_window_rows(); ncols = Rast_window_cols(); - bufrows = atoi(memory->answer) * (((1 << 20) / out_cell_size) / ncols); + /* memory reserved for presult and patch */ + in_buf_size = out_cell_size * ncols * nprocs * 2; + /* memory available for output buffer */ + out_buf_size = (size_t)atoi(memory->answer) * (1 << 20); + /* size_t is unsigned, check if any memory is left for output buffer */ + if (out_buf_size <= in_buf_size) + out_buf_size = 0; + else + out_buf_size -= in_buf_size; + /* number of buffered output rows */ + bufrows = out_buf_size / (out_cell_size * ncols); /* set the output buffer rows to be at most covering the entire map */ if (bufrows > nrows) { bufrows = nrows; diff --git a/raster/r.resamp.filter/main.c b/raster/r.resamp.filter/main.c index 559930cd5bf..137e2ac4b10 100644 --- a/raster/r.resamp.filter/main.c +++ b/raster/r.resamp.filter/main.c @@ -355,7 +355,6 @@ static void filter(void) if (row0 >= read_row && row0 < read_row + num_rows) { int m = row0 - read_row; int n = read_row + num_rows - row0; - int i; for (i = 0; i < n; i++) { DCELL *tmp = bufs[t_id][i]; @@ -415,6 +414,7 @@ int main(int argc, char *argv[]) char title[64]; int i, t; int nprocs; + size_t in_buf_size, out_buf_size; G_gisinit(argv[0]); @@ -623,11 +623,25 @@ int main(int argc, char *argv[]) Rast_set_input_window(&src_w); Rast_set_output_window(&dst_w); - bufrows = - atoi(parm.memory->answer) * (((1 << 20) / sizeof(DCELL)) / dst_w.cols); + /* memory reserved for input */ + in_buf_size = dst_w.cols * sizeof(DCELL) * row_scale * nprocs; + /* memory available for output buffer */ + out_buf_size = (size_t)atoi(parm.memory->answer) * (1 << 20); + /* size_t is unsigned, check if any memory is left for output buffer */ + if (out_buf_size <= in_buf_size) + out_buf_size = 0; + else + out_buf_size -= in_buf_size; + /* number of buffered output rows */ + bufrows = out_buf_size / (sizeof(DCELL) * dst_w.cols); + /* set the output buffer rows to be at most covering the entire map */ if (bufrows > dst_w.rows) { bufrows = dst_w.rows; } + /* but at least the number of threads */ + if (bufrows < nprocs) { + bufrows = nprocs; + } inbuf = G_malloc(nprocs * sizeof(DCELL *)); for (t = 0; t < nprocs; t++) diff --git a/raster/r.series/main.c b/raster/r.series/main.c index 4e0d360d265..8526bd6b1d9 100644 --- a/raster/r.series/main.c +++ b/raster/r.series/main.c @@ -125,6 +125,7 @@ int main(int argc, char *argv[]) int num_inputs; struct input **inputs = NULL; int bufrows; + size_t in_buf_size, out_buf_size; #if defined(_OPENMP) omp_lock_t fd_lock; @@ -399,16 +400,6 @@ int main(int argc, char *argv[]) nrows = Rast_window_rows(); ncols = Rast_window_cols(); - bufrows = atoi(parm.memory->answer) * (((1 << 20) / sizeof(DCELL)) / ncols); - /* set the output buffer rows to be at most covering the entire map */ - if (bufrows > nrows) { - bufrows = nrows; - } - /* but at least the number of threads */ - if (bufrows < nprocs) { - bufrows = nprocs; - } - /* set the locks for lazily opening raster files */ #if defined(_OPENMP) if (flag.lazy->answer && threaded) { @@ -429,6 +420,26 @@ int main(int argc, char *argv[]) outputs = G_calloc(num_outputs, sizeof(struct output)); + /* memory reserved for input */ + in_buf_size = ncols * sizeof(DCELL) * num_inputs * nprocs; + /* memory available for output buffer */ + out_buf_size = (size_t)atoi(parm.memory->answer) * (1 << 20); + /* size_t is unsigned, check if any memory is left for output buffer */ + if (out_buf_size <= in_buf_size) + out_buf_size = 0; + else + out_buf_size -= in_buf_size; + /* number of buffered rows for all output maps */ + bufrows = out_buf_size / (sizeof(DCELL) * ncols * num_outputs); + /* set the output buffer rows to be at most covering the entire map */ + if (bufrows > nrows) { + bufrows = nrows; + } + /* but at least the number of threads */ + if (bufrows < nprocs) { + bufrows = nprocs; + } + for (i = 0; i < num_outputs; i++) { struct output *out = &outputs[i]; const char *output_name = parm.output->answers[i];