From b3eed3d6fc849b9e16fbace1f37d401424f81ab0 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 25 Jan 2023 07:18:11 +0000 Subject: [PATCH v1] Review comments io_direct GUC --- src/backend/storage/file/fd.c | 57 ++++++++++++----------------- src/backend/utils/misc/guc_tables.c | 4 +- src/include/utils/guc_hooks.h | 1 - 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index eb83de4fb9..329acc2ffd 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -3749,7 +3749,7 @@ data_sync_elevel(int elevel) bool check_io_direct(char **newval, void **extra, GucSource source) { - int *flags = guc_malloc(ERROR, sizeof(*flags)); + int flags; #if PG_O_DIRECT == 0 if (strcmp(*newval, "") != 0) @@ -3757,38 +3757,51 @@ check_io_direct(char **newval, void **extra, GucSource source) GUC_check_errdetail("io_direct is not supported on this platform."); return false; } - *flags = 0; + flags = 0; #else - List *list; + List *elemlist; ListCell *l; + char *rawstring; - if (!SplitGUCList(*newval, ',', &list)) + /* Need a modifiable copy of string */ + rawstring = pstrdup(*newval); + + if (!SplitGUCList(rawstring, ',', &elemlist)) { GUC_check_errdetail("invalid list syntax in parameter \"%s\"", "io_direct"); + pfree(rawstring); + list_free(elemlist); return false; } - *flags = 0; - foreach (l, list) + flags = 0; + foreach (l, elemlist) { char *item = (char *) lfirst(l); if (pg_strcasecmp(item, "data") == 0) - *flags |= IO_DIRECT_DATA; + flags |= IO_DIRECT_DATA; else if (pg_strcasecmp(item, "wal") == 0) - *flags |= IO_DIRECT_WAL; + flags |= IO_DIRECT_WAL; else if (pg_strcasecmp(item, "wal_init") == 0) - *flags |= IO_DIRECT_WAL_INIT; + flags |= IO_DIRECT_WAL_INIT; else { GUC_check_errdetail("invalid option \"%s\"", item); + pfree(rawstring); + list_free(elemlist); return false; } } + + pfree(rawstring); + list_free(elemlist); #endif - *extra = flags; + /* Save the flags in *extra, for use by assign_io_direct */ + *extra = guc_malloc(ERROR, sizeof(int)); + *((int *) *extra) = flags; return true; } @@ -3800,27 +3813,3 @@ assign_io_direct(const char *newval, void *extra) io_direct_flags = *flags; } - -extern const char * -show_io_direct(void) -{ - static char result[80]; - - result[0] = 0; - if (io_direct_flags & IO_DIRECT_DATA) - strcat(result, "data"); - if (io_direct_flags & IO_DIRECT_WAL) - { - if (result[0]) - strcat(result, ", "); - strcat(result, "wal"); - } - if (io_direct_flags & IO_DIRECT_WAL_INIT) - { - if (result[0]) - strcat(result, ", "); - strcat(result, "wal_init"); - } - - return result; -} diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 9410493ae7..25b7e87abb 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -4529,11 +4529,11 @@ struct config_string ConfigureNamesString[] = {"io_direct", PGC_POSTMASTER, DEVELOPER_OPTIONS, gettext_noop("Use direct I/O for file access."), NULL, - GUC_NOT_IN_SAMPLE + GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE }, &io_direct_string, "", - check_io_direct, assign_io_direct, show_io_direct + check_io_direct, assign_io_direct, NULL }, /* End-of-list marker */ diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index e656a16a40..b3b5148185 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -156,6 +156,5 @@ extern void assign_wal_consistency_checking(const char *newval, void *extra); extern void assign_xlog_sync_method(int new_sync_method, void *extra); extern bool check_io_direct(char **newval, void **extra, GucSource source); extern void assign_io_direct(const char *newval, void *extra); -extern const char *show_io_direct(void); #endif /* GUC_HOOKS_H */ -- 2.34.1