[Cluster-devel] [PATCHv2 dlm/next] fs: dlm: log plock operation time
This patch adds more debug logging regarding to plock operation time.
Currently plocks are handled by user space and we are waiting for a
reply of the user space. To measure this upcall and downcall again we
introduce logging information of how much time the response took and
additional posix lock information to make a match with the user space
application.
Signed-off-by: Alexander Aring
---
changes since v2:
instead using owner we printout the pid taken from
fl->fl_pid which is necessary to match with userspace
application such as dlm_controld logging.
fs/dlm/plock.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index 737f185aad8d..d3115d66eece 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -175,6 +175,13 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
goto out;
}
+ log_debug(ls, "%s: op wait done %d %d %d %x %llx %llx-%llx %d %u",
+ __func__, fl->fl_pid, fl->fl_type, cmd,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(rv));
+
do_lock_wait:
WARN_ON(!list_empty(>list));
@@ -252,6 +259,7 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64
number, struct file *file,
struct plock_op *op;
int rv;
unsigned char fl_flags = fl->fl_flags;
+ unsigned long t;
ls = dlm_find_lockspace_local(lockspace);
if (!ls)
@@ -295,8 +303,16 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64
number, struct file *file,
}
send_op(op);
+ t = jiffies;
wait_event(recv_wq, (op->done != 0));
+ log_debug(ls, "%s: op wait done %d %d %x %llx %llx-%llx %d %u",
+ __func__, fl->fl_pid, fl->fl_type,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(jiffies - t));
+
WARN_ON(!list_empty(>list));
rv = op->info.rv;
@@ -318,6 +334,7 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
{
struct dlm_ls *ls;
struct plock_op *op;
+ unsigned long t;
int rv;
ls = dlm_find_lockspace_local(lockspace);
@@ -343,8 +360,16 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
op->info.owner = (__u64)(long) fl->fl_owner;
send_op(op);
+ t = jiffies;
wait_event(recv_wq, (op->done != 0));
+ log_debug(ls, "%s: op wait done %d %d %x %llx %llx-%llx %d %u",
+ __func__, fl->fl_pid, fl->fl_type,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(jiffies - t));
+
WARN_ON(!list_empty(>list));
/* info.rv from userspace is 1 for conflict, 0 for no-conflict,
--
2.31.1
[Cluster-devel] [PATCH dlm/next] fs: dlm: log plock operation time
This patch adds more debug logging regarding to plock operation time.
Currently plocks are handled by user space and we are waiting for a
reply of the user space. To measure this upcall and downcall again we
introduce logging information of how much time the response took and
additional posix lock information to make a match with the user space
application.
Signed-off-by: Alexander Aring
---
fs/dlm/plock.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index 737f185aad8d..0f36ea7f4fd6 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -175,6 +175,13 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
goto out;
}
+ log_debug(ls, "%s: op wait done %d %d %d %x %llx %llx-%llx %d %u",
+ __func__, fl->fl_pid, fl->fl_type, cmd,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(rv));
+
do_lock_wait:
WARN_ON(!list_empty(>list));
@@ -252,6 +259,7 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64
number, struct file *file,
struct plock_op *op;
int rv;
unsigned char fl_flags = fl->fl_flags;
+ unsigned long t;
ls = dlm_find_lockspace_local(lockspace);
if (!ls)
@@ -295,8 +303,16 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64
number, struct file *file,
}
send_op(op);
+ t = jiffies;
wait_event(recv_wq, (op->done != 0));
+ log_debug(ls, "%s: op wait done %llu %d %x %llx %llx-%llx %d %u",
+ __func__, (unsigned long long)op->info.owner, fl->fl_type,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(jiffies - t));
+
WARN_ON(!list_empty(>list));
rv = op->info.rv;
@@ -318,6 +334,7 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
{
struct dlm_ls *ls;
struct plock_op *op;
+ unsigned long t;
int rv;
ls = dlm_find_lockspace_local(lockspace);
@@ -343,8 +360,16 @@ int dlm_posix_get(dlm_lockspace_t *lockspace, u64 number,
struct file *file,
op->info.owner = (__u64)(long) fl->fl_owner;
send_op(op);
+ t = jiffies;
wait_event(recv_wq, (op->done != 0));
+ log_debug(ls, "%s: op wait done %llu %d %x %llx %llx-%llx %d %u",
+ __func__, (unsigned long long)op->info.owner, fl->fl_type,
+ ls->ls_global_id, (unsigned long long)number,
+ (unsigned long long)fl->fl_start,
+ (unsigned long long)fl->fl_end,
+ op->info.rv, jiffies_to_usecs(jiffies - t));
+
WARN_ON(!list_empty(>list));
/* info.rv from userspace is 1 for conflict, 0 for no-conflict,
--
2.31.1
Re: [Cluster-devel] [PATCH dlm-tool 6/8] dlm_controld: log lock/pending/waiter state changes
Hi,
On Mon, Jan 30, 2023 at 2:24 PM Alexander Aring wrote:
>
> This patch will trace lock state changes of the used dlm posix locks. In
> combination with the plock logfile we can see state changes over time
> and follow posix locks and their state.
> ---
> dlm_controld/dlm_daemon.h | 2 +-
> dlm_controld/logging.c| 2 +-
> dlm_controld/plock.c | 124 +-
> 3 files changed, 97 insertions(+), 31 deletions(-)
>
> diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
> index c74f684a..9bf3f621 100644
> --- a/dlm_controld/dlm_daemon.h
> +++ b/dlm_controld/dlm_daemon.h
> @@ -220,7 +220,7 @@ EXTERN struct list_head run_ops;
> #define LOG_PLOCK 0x0001
> #define LOG_NONE 0x
>
> -void log_level(char *name_in, uint32_t level_in, const char *fmt, ...);
> +void log_level(const char *name_in, uint32_t level_in, const char *fmt, ...);
>
> #define log_error(fmt, args...) log_level(NULL, LOG_ERR, fmt, ##args)
> #define log_debug(fmt, args...) log_level(NULL, LOG_DEBUG, fmt, ##args)
> diff --git a/dlm_controld/logging.c b/dlm_controld/logging.c
> index 83de2da4..e71fe52c 100644
> --- a/dlm_controld/logging.c
> +++ b/dlm_controld/logging.c
> @@ -175,7 +175,7 @@ static void log_str_to_file(FILE *fp)
> fflush(fp);
> }
>
> -void log_level(char *name_in, uint32_t level_in, const char *fmt, ...)
> +void log_level(const char *name_in, uint32_t level_in, const char *fmt, ...)
> {
> va_list ap;
> char name[NAME_ID_SIZE + 2];
> diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
> index 462c9212..77c043fd 100644
> --- a/dlm_controld/plock.c
> +++ b/dlm_controld/plock.c
> @@ -208,6 +208,62 @@ static uint64_t dt_usec(struct timeval *start, struct
> timeval *stop)
> return dt;
> }
>
> +static void plock_print_lock_add_state(const struct lockspace *ls,
> + const struct lock_waiter *w,
> + const char *state)
> +{
> + log_dlock(ls, "state: add %s %llx %llx-%llx %d/%u/%llx",
> + state,
> + (unsigned long long)w->info.number,
> + (unsigned long long)w->info.start,
> + (unsigned long long)w->info.end,
> + w->info.nodeid, w->info.pid,
> + (unsigned long long)w->info.owner);
> +}
> +
> +static void plock_print_lock_clear_state(const struct lockspace *ls,
> +const struct lock_waiter *w,
> +const char *state)
> +{
> + log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx",
> + state,
> + (unsigned long long)w->info.number,
> + (unsigned long long)w->info.start,
> + (unsigned long long)w->info.end,
> + w->info.nodeid, w->info.pid,
> + (unsigned long long)w->info.owner);
> +}
> +
> +#define plock_print_add_waiter(ls, w) \
> + plock_print_lock_add_state(ls, w, "waiter")
> +#define plock_print_clear_waiter(ls, w) \
> + plock_print_lock_clear_state(ls, w, "waiter")
> +
> +#define plock_print_add_pending(ls, w) \
> + plock_print_lock_add_state(ls, w, "pending")
> +#define plock_print_clear_pending(ls, w) \
> + plock_print_lock_clear_state(ls, w, "pending")
> +
> +static void plock_print_add_plock(const struct lockspace *ls,
> + const struct posix_lock *plock)
changes *plock to *po, because it's commonly used as *po in other
parts of the code for a struct posix_lock...
> +{
> + log_dlock(ls, "state: add plock NA %llx-%llx %d/%u/%llx",
removed this NA stuff, there is one attribute which plock doesn't have
but waiter has and I did this as placeholder... it's removed in 8/8 so
I drop to add it.
> + (unsigned long long)plock->start,
> + (unsigned long long)plock->end,
> + plock->nodeid, plock->pid,
> + (unsigned long long)plock->owner);
> +}
> +
> +static void plock_print_del_plock(const struct lockspace *ls,
> + const struct posix_lock *plock)
> +{
> + log_dlock(ls, "state: del plock NA %llx-%llx %d/%u/%llx",
> + (unsigned long long)plock->start,
> + (unsigned long long)plock->end,
> + plock->nodeid, plock->pid,
> + (unsigned long long)plock->owner);
> +}
same here.
- Alex
Re: [Cluster-devel] [PATCH dlm-tool 8/8] dlm_controld: add time diff for state time intervals
Hi,
On Mon, Jan 30, 2023 at 2:24 PM Alexander Aring wrote:
>
> This patch adds functionality to see how long a posix lock is alive or
> is in waiting or pending state. It can be used to filter out interesting
> locks which are stuck in e.g. waiting state to know that a user space
> process probably has contention on it. The logging information will
> printout additional lock information to do more search and find to get
> more information about it's corosync or kernel communication.
> ---
> dlm_controld/plock.c | 48 ++--
> 1 file changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
> index d83a79d2..20c2a1e9 100644
> --- a/dlm_controld/plock.c
> +++ b/dlm_controld/plock.c
> @@ -72,6 +72,7 @@ struct resource {
>
> struct posix_lock {
> struct list_headlist; /* resource locks or waiters list
> */
> + struct timeval list_time;
> uint32_tpid;
> uint64_towner;
> uint64_tstart;
> @@ -83,6 +84,7 @@ struct posix_lock {
>
> struct lock_waiter {
> struct list_headlist;
> + struct timeval list_time;
> uint32_tflags;
> struct dlm_plock_info info;
> };
> @@ -209,9 +211,11 @@ static uint64_t dt_usec(const struct timeval *start,
> const struct timeval *stop)
> }
>
> static void plock_print_lock_add_state(const struct lockspace *ls,
> - const struct lock_waiter *w,
> + struct lock_waiter *w,
>const char *state)
> {
> + gettimeofday(>list_time, NULL);
> +
> log_dlock(ls, "state: add %s %llx %llx-%llx %d/%u/%llx",
> state,
> (unsigned long long)w->info.number,
> @@ -225,13 +229,20 @@ static void plock_print_lock_clear_state(const struct
> lockspace *ls,
> const struct lock_waiter *w,
> const char *state)
> {
> - log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx",
> + struct timeval now;
> + uint64_t usec;
> +
> + gettimeofday(, NULL);
> + usec = dt_usec(>list_time, );
> +
> + log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx %.3f",
> state,
> (unsigned long long)w->info.number,
> (unsigned long long)w->info.start,
> (unsigned long long)w->info.end,
> w->info.nodeid, w->info.pid,
> - (unsigned long long)w->info.owner);
> + (unsigned long long)w->info.owner,
> + usec * 1.e-6);
> }
>
> #define plock_print_add_waiter(ls, w) \
> @@ -245,23 +256,30 @@ static void plock_print_lock_clear_state(const struct
> lockspace *ls,
> plock_print_lock_clear_state(ls, w, "pending")
>
> static void plock_print_add_plock(const struct lockspace *ls,
> - const struct posix_lock *plock)
> + struct posix_lock *po)
> {
> - log_dlock(ls, "state: add plock NA %llx-%llx %d/%u/%llx",
> - (unsigned long long)plock->start,
> - (unsigned long long)plock->end,
> - plock->nodeid, plock->pid,
> - (unsigned long long)plock->owner);
> + gettimeofday(>list_time, NULL);
> + log_dlock(ls, "state: add plock %llx-%llx %d/%u/%llx %.3f",
removed "%.3f" for plock_print_add_plock(), it's only necessary for
clear. We probably need to add some format(printf, 2, 3) gcc attribute
to see such mistakes...
- Alex
[Cluster-devel] [PATCH dlm-tool 8/8] dlm_controld: add time diff for state time intervals
This patch adds functionality to see how long a posix lock is alive or
is in waiting or pending state. It can be used to filter out interesting
locks which are stuck in e.g. waiting state to know that a user space
process probably has contention on it. The logging information will
printout additional lock information to do more search and find to get
more information about it's corosync or kernel communication.
---
dlm_controld/plock.c | 48 ++--
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
index d83a79d2..20c2a1e9 100644
--- a/dlm_controld/plock.c
+++ b/dlm_controld/plock.c
@@ -72,6 +72,7 @@ struct resource {
struct posix_lock {
struct list_headlist; /* resource locks or waiters list */
+ struct timeval list_time;
uint32_tpid;
uint64_towner;
uint64_tstart;
@@ -83,6 +84,7 @@ struct posix_lock {
struct lock_waiter {
struct list_headlist;
+ struct timeval list_time;
uint32_tflags;
struct dlm_plock_info info;
};
@@ -209,9 +211,11 @@ static uint64_t dt_usec(const struct timeval *start, const
struct timeval *stop)
}
static void plock_print_lock_add_state(const struct lockspace *ls,
- const struct lock_waiter *w,
+ struct lock_waiter *w,
const char *state)
{
+ gettimeofday(>list_time, NULL);
+
log_dlock(ls, "state: add %s %llx %llx-%llx %d/%u/%llx",
state,
(unsigned long long)w->info.number,
@@ -225,13 +229,20 @@ static void plock_print_lock_clear_state(const struct
lockspace *ls,
const struct lock_waiter *w,
const char *state)
{
- log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx",
+ struct timeval now;
+ uint64_t usec;
+
+ gettimeofday(, NULL);
+ usec = dt_usec(>list_time, );
+
+ log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx %.3f",
state,
(unsigned long long)w->info.number,
(unsigned long long)w->info.start,
(unsigned long long)w->info.end,
w->info.nodeid, w->info.pid,
- (unsigned long long)w->info.owner);
+ (unsigned long long)w->info.owner,
+ usec * 1.e-6);
}
#define plock_print_add_waiter(ls, w) \
@@ -245,23 +256,30 @@ static void plock_print_lock_clear_state(const struct
lockspace *ls,
plock_print_lock_clear_state(ls, w, "pending")
static void plock_print_add_plock(const struct lockspace *ls,
- const struct posix_lock *plock)
+ struct posix_lock *po)
{
- log_dlock(ls, "state: add plock NA %llx-%llx %d/%u/%llx",
- (unsigned long long)plock->start,
- (unsigned long long)plock->end,
- plock->nodeid, plock->pid,
- (unsigned long long)plock->owner);
+ gettimeofday(>list_time, NULL);
+ log_dlock(ls, "state: add plock %llx-%llx %d/%u/%llx %.3f",
+ (unsigned long long)po->start,
+ (unsigned long long)po->end,
+ po->nodeid, po->pid,
+ (unsigned long long)po->owner);
}
static void plock_print_del_plock(const struct lockspace *ls,
- const struct posix_lock *plock)
+ const struct posix_lock *po)
{
- log_dlock(ls, "state: del plock NA %llx-%llx %d/%u/%llx",
- (unsigned long long)plock->start,
- (unsigned long long)plock->end,
- plock->nodeid, plock->pid,
- (unsigned long long)plock->owner);
+ struct timeval now;
+ uint64_t usec;
+
+ gettimeofday(, NULL);
+ usec = dt_usec(>list_time, );
+ log_dlock(ls, "state: clear plock %llx-%llx %d/%u/%llx %.3f",
+ (unsigned long long)po->start,
+ (unsigned long long)po->end,
+ po->nodeid, po->pid,
+ (unsigned long long)po->owner,
+ usec * 1.e-6);
}
static struct resource * rb_search_plock_resource(struct lockspace *ls,
uint64_t number)
--
2.31.1
[Cluster-devel] [PATCH dlm-tool 3/8] dlm_controld: add plock logfile
The current plock logging is limited due a in-memory log buffer which
can be dumped via dlm_contol log_plock functionality. To trace plock
performance issues it's necessary to log plock activity in a bigger log
buffer such as a file. This patch will add functionality that plock
logging information will be appended into a log file.
WARNING: depending on plock activity the resulting log file can be
resulting in enormous file size. This option should be used for
debugging purpose only.
---
dlm_controld/dlm_daemon.h | 4
dlm_controld/logging.c| 39 +++
dlm_controld/main.c | 5 +
3 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
index f0bad90f..c74f684a 100644
--- a/dlm_controld/dlm_daemon.h
+++ b/dlm_controld/dlm_daemon.h
@@ -76,10 +76,12 @@
#define RUN_FILE_NAME"dlm_controld.pid"
#define LOG_FILE_NAME"dlm_controld.log"
+#define PLOCK_LOG_FILE_NAME "plock.log"
#define CONF_FILE_NAME "dlm.conf"
#define RUN_FILE_PATHRUNDIR "/" RUN_FILE_NAME
#define LOG_FILE_PATHLOGDIR "/" LOG_FILE_NAME
+#define PLOCK_LOG_FILE_PATH LOGDIR "/" PLOCK_LOG_FILE_NAME
#define CONF_FILE_PATH CONFDIR "/" CONF_FILE_NAME
#define DEFAULT_LOG_MODE LOG_MODE_OUTPUT_FILE | LOG_MODE_OUTPUT_SYSLOG
@@ -87,6 +89,7 @@
#define DEFAULT_SYSLOG_PRIORITY LOG_INFO
#define DEFAULT_LOGFILE_PRIORITY LOG_INFO
#define DEFAULT_LOGFILE LOG_FILE_PATH
+#define DEFAULT_PLOCK_LOGFILE PLOCK_LOG_FILE_PATH
#define DEFAULT_NETLINK_RCVBUF (2 * 1024 * 1024)
@@ -110,6 +113,7 @@ enum {
enable_fscontrol_ind,
enable_plock_ind,
plock_debug_ind,
+plock_debug_logfile_ind,
plock_rate_limit_ind,
plock_ownership_ind,
drop_resources_time_ind,
diff --git a/dlm_controld/logging.c b/dlm_controld/logging.c
index 3298ef99..83de2da4 100644
--- a/dlm_controld/logging.c
+++ b/dlm_controld/logging.c
@@ -12,7 +12,9 @@ static int syslog_facility;
static int syslog_priority;
static int logfile_priority;
static char logfile[PATH_MAX];
+static char plock_logfile[PATH_MAX];
static FILE *logfile_fp;
+static FILE *plock_logfile_fp;
/* logfile_priority is the only one of these options that
can be controlled from command line, environment variable
@@ -35,6 +37,7 @@ void init_logging(void)
syslog_priority = DEFAULT_SYSLOG_PRIORITY;
logfile_priority = DEFAULT_LOGFILE_PRIORITY;
strcpy(logfile, DEFAULT_LOGFILE);
+ strcpy(plock_logfile, DEFAULT_PLOCK_LOGFILE);
set_logfile_priority();
@@ -66,6 +69,15 @@ void init_logging(void)
}
}
+ if (dlm_options[plock_debug_logfile_ind].use_int &&
+ plock_logfile[0]) {
+ plock_logfile_fp = fopen(plock_logfile, "a+");
+ if (plock_logfile_fp != NULL) {
+ int fd = fileno(plock_logfile_fp);
+ fcntl(fd, F_SETFD, fcntl(fd, F_GETFD, 0) | FD_CLOEXEC);
+ }
+ }
+
skip_logfile:
openlog(DAEMON_NAME, LOG_CONS | LOG_PID, syslog_facility);
}
@@ -75,6 +87,8 @@ void close_logging(void)
closelog();
if (logfile_fp)
fclose(logfile_fp);
+ if (plock_logfile_fp)
+ fclose(plock_logfile_fp);
}
#define NAME_ID_SIZE 32
@@ -151,6 +165,16 @@ static void log_save_str(int len, char *log_buf, unsigned
int *point,
*wrap = w;
}
+static void log_str_to_file(FILE *fp)
+{
+ time_t logtime = time(NULL);
+ char tbuf[64];
+
+ strftime(tbuf, sizeof(tbuf), "%b %d %T", localtime());
+ fprintf(fp, "%s %s", tbuf, log_str);
+ fflush(fp);
+}
+
void log_level(char *name_in, uint32_t level_in, const char *fmt, ...)
{
va_list ap;
@@ -191,19 +215,18 @@ void log_level(char *name_in, uint32_t level_in, const
char *fmt, ...)
if (level < LOG_NONE)
log_save_str(pos - 1, log_dump, _point, _wrap);
- if (plock)
+ if (plock) {
log_save_str(pos - 1, log_dump_plock, _point_plock,
_wrap_plock);
+ if (plock_logfile_fp)
+ log_str_to_file(plock_logfile_fp);
+ }
+
if (level <= syslog_priority)
syslog(level, "%s", log_str);
- if (level <= logfile_priority && logfile_fp) {
- time_t logtime = time(NULL);
- char tbuf[64];
- strftime(tbuf, sizeof(tbuf), "%b %d %T", localtime());
- fprintf(logfile_fp, "%s %s", tbuf, log_str);
- fflush(logfile_fp);
- }
+ if (level <= logfile_priority && logfile_fp)
+ log_str_to_file(logfile_fp);
if (!dlm_options[daemon_debug_ind].use_int)
return;
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index c9d1c5f1..8e8d4038 100644
--- a/dlm_controld/main.c
[Cluster-devel] [PATCH dlm-tool 1/8] dlm_tool: add fail functionality if dump failed
Currently dlm_controld sets a embedded data int value of dlm_controld
dump functionality failed for e.g. if lockspace cannot be found. The
dlm_tool does not parse this possible error functionality and will exit
successfully. This patch will add dlm_tool fail functionality if
dlm_controld sets an embedded data error value.
---
dlm_controld/lib.c | 25 +---
dlm_controld/libdlmcontrol.h | 10 +++
dlm_tool/main.c | 57 +++-
3 files changed, 62 insertions(+), 30 deletions(-)
diff --git a/dlm_controld/lib.c b/dlm_controld/lib.c
index 2888ad05..a21150f2 100644
--- a/dlm_controld/lib.c
+++ b/dlm_controld/lib.c
@@ -109,7 +109,7 @@ static void init_header(struct dlmc_header *h, int cmd,
char *name,
static char copy_buf[DLMC_DUMP_SIZE];
-static int do_dump(int cmd, char *name, char *buf)
+static int do_dump(int cmd, char *name, char *buf, int *data)
{
struct dlmc_header h;
int fd, rv, len;
@@ -118,6 +118,8 @@ static int do_dump(int cmd, char *name, char *buf)
init_header(, cmd, name, 0);
+ *data = 0;
+
fd = do_connect(DLMC_QUERY_SOCK_PATH);
if (fd < 0) {
rv = fd;
@@ -134,6 +136,7 @@ static int do_dump(int cmd, char *name, char *buf)
if (rv < 0)
goto out_close;
+ *data = h.data;
len = h.len - sizeof(h);
if (len <= 0 || len > DLMC_DUMP_SIZE)
@@ -150,29 +153,29 @@ static int do_dump(int cmd, char *name, char *buf)
return rv;
}
-int dlmc_dump_debug(char *buf)
+int dlmc_dump_debug(char *buf, int *data)
{
- return do_dump(DLMC_CMD_DUMP_DEBUG, NULL, buf);
+ return do_dump(DLMC_CMD_DUMP_DEBUG, NULL, buf, data);
}
-int dlmc_dump_config(char *buf)
+int dlmc_dump_config(char *buf, int *data)
{
- return do_dump(DLMC_CMD_DUMP_CONFIG, NULL, buf);
+ return do_dump(DLMC_CMD_DUMP_CONFIG, NULL, buf, data);
}
-int dlmc_dump_log_plock(char *buf)
+int dlmc_dump_log_plock(char *buf, int *data)
{
- return do_dump(DLMC_CMD_DUMP_LOG_PLOCK, NULL, buf);
+ return do_dump(DLMC_CMD_DUMP_LOG_PLOCK, NULL, buf, data);
}
-int dlmc_dump_plocks(char *name, char *buf)
+int dlmc_dump_plocks(char *name, char *buf, int *data)
{
- return do_dump(DLMC_CMD_DUMP_PLOCKS, name, buf);
+ return do_dump(DLMC_CMD_DUMP_PLOCKS, name, buf, data);
}
-int dlmc_dump_run(char *buf)
+int dlmc_dump_run(char *buf, int *data)
{
- return do_dump(DLMC_CMD_DUMP_RUN, NULL, buf);
+ return do_dump(DLMC_CMD_DUMP_RUN, NULL, buf, data);
}
int dlmc_reload_config(void)
diff --git a/dlm_controld/libdlmcontrol.h b/dlm_controld/libdlmcontrol.h
index a8654f3e..08f04c39 100644
--- a/dlm_controld/libdlmcontrol.h
+++ b/dlm_controld/libdlmcontrol.h
@@ -80,11 +80,11 @@ struct dlmc_lockspace {
#define DLMC_STATUS_VERBOSE0x0001
-int dlmc_dump_debug(char *buf);
-int dlmc_dump_config(char *buf);
-int dlmc_dump_run(char *buf);
-int dlmc_dump_log_plock(char *buf);
-int dlmc_dump_plocks(char *name, char *buf);
+int dlmc_dump_debug(char *buf, int *data);
+int dlmc_dump_config(char *buf, int *data);
+int dlmc_dump_run(char *buf, int *data);
+int dlmc_dump_log_plock(char *buf, int *data);
+int dlmc_dump_plocks(char *name, char *buf, int *data);
int dlmc_lockspace_info(char *lsname, struct dlmc_lockspace *ls);
int dlmc_node_info(char *lsname, int nodeid, struct dlmc_node *node);
/* caller need to free *lss */
diff --git a/dlm_tool/main.c b/dlm_tool/main.c
index 50f0cae9..52fd5b89 100644
--- a/dlm_tool/main.c
+++ b/dlm_tool/main.c
@@ -1466,36 +1466,51 @@ static void do_fence_ack(char *name)
dlmc_fence_ack(name);
}
-static void do_plocks(char *name)
+static int do_plocks(char *name)
{
char buf[DLMC_DUMP_SIZE];
+ int rv, data;
memset(buf, 0, sizeof(buf));
- dlmc_dump_plocks(name, buf);
+ rv = dlmc_dump_plocks(name, buf, );
+ if (rv)
+ return rv;
+ else if (data)
+ return data;
buf[DLMC_DUMP_SIZE-1] = '\0';
do_write(STDOUT_FILENO, buf, strlen(buf));
+
+ return 0;
}
-static void do_dump(int op)
+static int do_dump(int op)
{
+ int rv = -EOPNOTSUPP, data;
char buf[DLMC_DUMP_SIZE];
memset(buf, 0, sizeof(buf));
if (op == OP_DUMP)
- dlmc_dump_debug(buf);
+ rv = dlmc_dump_debug(buf, );
else if (op == OP_DUMP_CONFIG)
- dlmc_dump_config(buf);
+ rv = dlmc_dump_config(buf, );
else if (op == OP_DUMP_RUN)
- dlmc_dump_run(buf);
+ rv = dlmc_dump_run(buf, );
+
+ if (rv)
+ return rv;
+ else if (data)
+ return data;
buf[DLMC_DUMP_SIZE-1] = '\0';
do_write(STDOUT_FILENO, buf, strlen(buf));
printf("\n");
+
+ return 0;
}
static void do_reload_config(void)
@@ -1514,18 +1529,25 @@ static void
[Cluster-devel] [PATCH dlm-tool 7/8] dlm_controld: constify timeval of dt_usec()
Those parameters are only used read only. We don't change any data where
those pointers point to.
---
dlm_controld/plock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
index 77c043fd..d83a79d2 100644
--- a/dlm_controld/plock.c
+++ b/dlm_controld/plock.c
@@ -198,7 +198,7 @@ static unsigned long time_diff_ms(struct timeval *begin,
struct timeval *end)
return (result.tv_sec * 1000) + (result.tv_usec / 1000);
}
-static uint64_t dt_usec(struct timeval *start, struct timeval *stop)
+static uint64_t dt_usec(const struct timeval *start, const struct timeval
*stop)
{
uint64_t dt;
--
2.31.1
[Cluster-devel] [PATCH dlm-tool 6/8] dlm_controld: log lock/pending/waiter state changes
This patch will trace lock state changes of the used dlm posix locks. In
combination with the plock logfile we can see state changes over time
and follow posix locks and their state.
---
dlm_controld/dlm_daemon.h | 2 +-
dlm_controld/logging.c| 2 +-
dlm_controld/plock.c | 124 +-
3 files changed, 97 insertions(+), 31 deletions(-)
diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
index c74f684a..9bf3f621 100644
--- a/dlm_controld/dlm_daemon.h
+++ b/dlm_controld/dlm_daemon.h
@@ -220,7 +220,7 @@ EXTERN struct list_head run_ops;
#define LOG_PLOCK 0x0001
#define LOG_NONE 0x
-void log_level(char *name_in, uint32_t level_in, const char *fmt, ...);
+void log_level(const char *name_in, uint32_t level_in, const char *fmt, ...);
#define log_error(fmt, args...) log_level(NULL, LOG_ERR, fmt, ##args)
#define log_debug(fmt, args...) log_level(NULL, LOG_DEBUG, fmt, ##args)
diff --git a/dlm_controld/logging.c b/dlm_controld/logging.c
index 83de2da4..e71fe52c 100644
--- a/dlm_controld/logging.c
+++ b/dlm_controld/logging.c
@@ -175,7 +175,7 @@ static void log_str_to_file(FILE *fp)
fflush(fp);
}
-void log_level(char *name_in, uint32_t level_in, const char *fmt, ...)
+void log_level(const char *name_in, uint32_t level_in, const char *fmt, ...)
{
va_list ap;
char name[NAME_ID_SIZE + 2];
diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
index 462c9212..77c043fd 100644
--- a/dlm_controld/plock.c
+++ b/dlm_controld/plock.c
@@ -208,6 +208,62 @@ static uint64_t dt_usec(struct timeval *start, struct
timeval *stop)
return dt;
}
+static void plock_print_lock_add_state(const struct lockspace *ls,
+ const struct lock_waiter *w,
+ const char *state)
+{
+ log_dlock(ls, "state: add %s %llx %llx-%llx %d/%u/%llx",
+ state,
+ (unsigned long long)w->info.number,
+ (unsigned long long)w->info.start,
+ (unsigned long long)w->info.end,
+ w->info.nodeid, w->info.pid,
+ (unsigned long long)w->info.owner);
+}
+
+static void plock_print_lock_clear_state(const struct lockspace *ls,
+const struct lock_waiter *w,
+const char *state)
+{
+ log_dlock(ls, "state: clear %s %llx %llx-%llx %d/%u/%llx",
+ state,
+ (unsigned long long)w->info.number,
+ (unsigned long long)w->info.start,
+ (unsigned long long)w->info.end,
+ w->info.nodeid, w->info.pid,
+ (unsigned long long)w->info.owner);
+}
+
+#define plock_print_add_waiter(ls, w) \
+ plock_print_lock_add_state(ls, w, "waiter")
+#define plock_print_clear_waiter(ls, w) \
+ plock_print_lock_clear_state(ls, w, "waiter")
+
+#define plock_print_add_pending(ls, w) \
+ plock_print_lock_add_state(ls, w, "pending")
+#define plock_print_clear_pending(ls, w) \
+ plock_print_lock_clear_state(ls, w, "pending")
+
+static void plock_print_add_plock(const struct lockspace *ls,
+ const struct posix_lock *plock)
+{
+ log_dlock(ls, "state: add plock NA %llx-%llx %d/%u/%llx",
+ (unsigned long long)plock->start,
+ (unsigned long long)plock->end,
+ plock->nodeid, plock->pid,
+ (unsigned long long)plock->owner);
+}
+
+static void plock_print_del_plock(const struct lockspace *ls,
+ const struct posix_lock *plock)
+{
+ log_dlock(ls, "state: del plock NA %llx-%llx %d/%u/%llx",
+ (unsigned long long)plock->start,
+ (unsigned long long)plock->end,
+ plock->nodeid, plock->pid,
+ (unsigned long long)plock->owner);
+}
+
static struct resource * rb_search_plock_resource(struct lockspace *ls,
uint64_t number)
{
struct rb_node *n = ls->plock_resources_root.rb_node;
@@ -447,8 +503,9 @@ static int is_conflict(struct resource *r, struct
dlm_plock_info *in, int get)
return 0;
}
-static int add_lock(struct resource *r, uint32_t nodeid, uint64_t owner,
- uint32_t pid, int ex, uint64_t start, uint64_t end)
+static int add_lock(const struct lockspace *ls, struct resource *r,
+ uint32_t nodeid, uint64_t owner, uint32_t pid,
+ int ex, uint64_t start, uint64_t end)
{
struct posix_lock *po;
@@ -464,6 +521,7 @@ static int add_lock(struct resource *r, uint32_t nodeid,
uint64_t owner,
po->pid = pid;
po->ex = ex;
list_add_tail(>list, >locks);
+ plock_print_add_plock(ls, po);
return 0;
}
@@ -472,8 +530,8 @@ static int add_lock(struct resource *r, uint32_t nodeid,
uint64_t owner,
1. add new lock for non-overlap area of
[Cluster-devel] [PATCH dlm-tool 5/8] dlm_controld: remove ls parameter
The ls parameter in write_result() is not used, so we can remove it.
---
dlm_controld/plock.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c
index c2c80360..462c9212 100644
--- a/dlm_controld/plock.c
+++ b/dlm_controld/plock.c
@@ -684,8 +684,7 @@ static int add_waiter(struct lockspace *ls, struct resource
*r,
return 0;
}
-static void write_result(struct lockspace *ls, struct dlm_plock_info *in,
-int rv)
+static void write_result(struct dlm_plock_info *in, int rv)
{
int write_rv;
@@ -719,7 +718,7 @@ static void do_waiters(struct lockspace *ls, struct
resource *r)
rv = lock_internal(ls, r, in);
if (in->nodeid == our_nodeid)
- write_result(ls, in, rv);
+ write_result(in, rv);
free(w);
}
@@ -744,7 +743,7 @@ static void do_lock(struct lockspace *ls, struct
dlm_plock_info *in,
out:
if (in->nodeid == our_nodeid && rv != -EINPROGRESS)
- write_result(ls, in, rv);
+ write_result(in, rv);
do_waiters(ls, r);
put_resource(ls, r);
@@ -768,7 +767,7 @@ static void do_unlock(struct lockspace *ls, struct
dlm_plock_info *in,
}
if (in->nodeid == our_nodeid)
- write_result(ls, in, rv);
+ write_result(in, rv);
skip_result:
do_waiters(ls, r);
@@ -787,7 +786,7 @@ static void do_get(struct lockspace *ls, struct
dlm_plock_info *in,
else
rv = 0;
- write_result(ls, in, rv);
+ write_result(in, rv);
put_resource(ls, r);
}
@@ -830,7 +829,7 @@ static void __receive_plock(struct lockspace *ls, struct
dlm_plock_info *in,
log_elock(ls, "receive_plock error from %d optype %d",
from, in->optype);
if (from == our_nodeid)
- write_result(ls, in, -EINVAL);
+ write_result(in, -EINVAL);
}
}
--
2.31.1
[Cluster-devel] [PATCH dlm-tool 2/8] dlm_controld: always create logdir
Currently the logdir will be created only if logfile does contain a
string. To add another logfiles we simple create the logdir always on
startup.
---
dlm_controld/logging.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/dlm_controld/logging.c b/dlm_controld/logging.c
index 2c57138c..3298ef99 100644
--- a/dlm_controld/logging.c
+++ b/dlm_controld/logging.c
@@ -38,27 +38,27 @@ void init_logging(void)
set_logfile_priority();
- if (logfile[0]) {
- old_umask = umask(0077);
- rv = mkdir(SYS_VARDIR, 0700);
- if (rv < 0 && errno != EEXIST) {
- umask(old_umask);
- goto skip_logfile;
- }
+ old_umask = umask(0077);
+ rv = mkdir(SYS_VARDIR, 0700);
+ if (rv < 0 && errno != EEXIST) {
+ umask(old_umask);
+ goto skip_logfile;
+ }
- rv = mkdir(SYS_LOGDIR, 0700);
- if (rv < 0 && errno != EEXIST) {
- umask(old_umask);
- goto skip_logfile;
- }
+ rv = mkdir(SYS_LOGDIR, 0700);
+ if (rv < 0 && errno != EEXIST) {
+ umask(old_umask);
+ goto skip_logfile;
+ }
- rv = mkdir(LOGDIR, 0700);
- if (rv < 0 && errno != EEXIST) {
- umask(old_umask);
- goto skip_logfile;
- }
+ rv = mkdir(LOGDIR, 0700);
+ if (rv < 0 && errno != EEXIST) {
umask(old_umask);
+ goto skip_logfile;
+ }
+ umask(old_umask);
+ if (logfile[0]) {
logfile_fp = fopen(logfile, "a+");
if (logfile_fp != NULL) {
int fd = fileno(logfile_fp);
--
2.31.1
[Cluster-devel] [PATCH dlm-tool 4/8] dlm_controld: move processing of saved messages to plock level
Ad the loglevel is for save plock messages during corosync resource membership upate. This patch will put the processing of saved messages on the same loglevel. --- dlm_controld/plock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlm_controld/plock.c b/dlm_controld/plock.c index 692787e2..c2c80360 100644 --- a/dlm_controld/plock.c +++ b/dlm_controld/plock.c @@ -1612,7 +1612,7 @@ void process_saved_plocks(struct lockspace *ls) struct dlm_header *hd; int count = 0; - log_dlock(ls, "process_saved_plocks begin"); + log_plock(ls, "process_saved_plocks begin"); if (list_empty(>saved_messages)) goto out; @@ -1643,7 +1643,7 @@ void process_saved_plocks(struct lockspace *ls) count++; } out: - log_dlock(ls, "process_saved_plocks %d done", count); + log_plock(ls, "process_saved_plocks %d done", count); } /* locks still marked SYNCING should not go into the ckpt; the new node -- 2.31.1
[Cluster-devel] [PATCH 0/6] gfs2-utils: Cleanups and fsck.gfs2 fixes
This set is mainly cleanups but patches 5 and 6 are small fixes for fsck.gfs2 which I would like to draw attention to. PR is https://pagure.io/gfs2-utils/pull-request/14 awaiting CI testing should you prefer to comment there. Andy Andrew Price (6): libgfs2: Return the inode from lgfs2_lookupi() libgfs2: Remove lgfs2_gfs_createi() libgfs2: Reorganise lgfs2_createi() fsck.gfs2: Remove de variable from dirref_find() fsck.gfs2: Fix wrong entry used in dentry comparison fsck.gfs2: fix_hashtable: Decrement i_blocks when freeing leaf blocks gfs2/convert/gfs2_convert.c | 7 +-- gfs2/edit/hexedit.c | 2 +- gfs2/fsck/fs_recovery.c | 14 ++--- gfs2/fsck/initialize.c | 18 +++--- gfs2/fsck/lost_n_found.c| 6 +- gfs2/fsck/pass2.c | 22 +++ gfs2/glocktop/glocktop.c| 6 +- gfs2/libgfs2/check_fs_ops.c | 18 +++--- gfs2/libgfs2/fs_ops.c | 121 +++- gfs2/libgfs2/gfs2l.c| 2 +- gfs2/libgfs2/lang.c | 10 +-- gfs2/libgfs2/libgfs2.h | 6 +- tests/nukerg.c | 2 +- 13 files changed, 113 insertions(+), 121 deletions(-) -- 2.39.0
[Cluster-devel] [PATCH 4/6] fsck.gfs2: Remove de variable from dirref_find()
It always points to 'dentry' so it can be replaced in all uses.
Signed-off-by: Andrew Price
---
gfs2/fsck/pass2.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 0c2e0146..002bf2cb 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -621,18 +621,17 @@ static int dirref_find(struct fsck_cx *cx, struct
lgfs2_inode *ip, struct gfs2_d
/* the metawalk_fxn's private field must be set to the dentry
* block we want to clear */
struct lgfs2_inum *entry = private;
- struct lgfs2_dirent dentry, *de;
+ struct lgfs2_dirent dentry;
char fn[MAX_FILENAME];
memset(, 0, sizeof(dentry));
lgfs2_dirent_in(, dent);
- de =
- if (de->dr_inum.in_addr != entry->in_addr) {
+ if (dentry.dr_inum.in_addr != entry->in_addr) {
(*count)++;
return 0;
}
- if (de->dr_inum.in_formal_ino ==
be64_to_cpu(dent->de_inum.no_formal_ino)) {
+ if (dentry.dr_inum.in_formal_ino ==
be64_to_cpu(dent->de_inum.no_formal_ino)) {
log_debug("Formal inode number matches; must be a hard "
"link.\n");
goto out;
@@ -641,13 +640,13 @@ static int dirref_find(struct fsck_cx *cx, struct
lgfs2_inode *ip, struct gfs2_d
"directory %"PRIu64" (0x%"PRIx64") has the wrong 'formal'
inode "
"number.\n"), entry->in_addr, entry->in_addr,
ip->i_num.in_addr, ip->i_num.in_addr);
memset(fn, 0, sizeof(fn));
- if (de->dr_name_len < MAX_FILENAME)
- strncpy(fn, filename, de->dr_name_len);
+ if (dentry.dr_name_len < MAX_FILENAME)
+ strncpy(fn, filename, dentry.dr_name_len);
else
strncpy(fn, filename, MAX_FILENAME - 1);
log_err(_("The bad reference '%s' had formal inode number: %"PRIu64
" (0x%"PRIx64") but the correct value is: %"PRIu64"
(0x%"PRIx64")\n"),
- fn, de->dr_inum.in_formal_ino, de->dr_inum.in_formal_ino,
+ fn, dentry.dr_inum.in_formal_ino, dentry.dr_inum.in_formal_ino,
entry->in_formal_ino, entry->in_formal_ino);
if (!query(cx, _("Delete the bad directory entry? (y/n) "))) {
log_err(_("The corrupt directory entry was not fixed.\n"));
--
2.39.0
[Cluster-devel] [PATCH 6/6] fsck.gfs2: fix_hashtable: Decrement i_blocks when freeing leaf blocks
fsck.gfs2 can leave i_blocks too large when it removes empty leaf
blocks, so decrease the count when freeing them.
Signed-off-by: Andrew Price
---
gfs2/fsck/pass2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 5327ee43..71459978 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1334,6 +1334,7 @@ static int fix_hashtable(struct fsck_cx *cx, struct
lgfs2_inode *ip, __be64 *tbl
(dentry.dr_inum.in_formal_ino == 0)) {
lgfs2_brelse(lbh);
lgfs2_free_block(ip->i_sbd, leafblk);
+ ip->i_blocks--;
log_err(_("Out of place leaf block %"PRIu64" (0x%"PRIx64") had
no "
"entries, so it was deleted.\n"),
leafblk, leafblk);
--
2.39.0
[Cluster-devel] [PATCH 5/6] fsck.gfs2: Fix wrong entry used in dentry comparison
'dent' points to the on-disk data that 'dentry' was converted from, so
this comparison always evaluates to true. Compare to 'entry' instead.
Signed-off-by: Andrew Price
---
gfs2/fsck/pass2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 002bf2cb..5327ee43 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -631,7 +631,7 @@ static int dirref_find(struct fsck_cx *cx, struct
lgfs2_inode *ip, struct gfs2_d
(*count)++;
return 0;
}
- if (dentry.dr_inum.in_formal_ino ==
be64_to_cpu(dent->de_inum.no_formal_ino)) {
+ if (dentry.dr_inum.in_formal_ino == entry->in_formal_ino) {
log_debug("Formal inode number matches; must be a hard "
"link.\n");
goto out;
--
2.39.0
[Cluster-devel] [PATCH 1/6] libgfs2: Return the inode from lgfs2_lookupi()
Instead of returning a -errno with the inode set via a passed in
pointer, return the inode and use errno. This is more consistent and
simplifies some calling code.
Signed-off-by: Andrew Price
---
gfs2/convert/gfs2_convert.c | 7 +++
gfs2/edit/hexedit.c | 2 +-
gfs2/fsck/fs_recovery.c | 14 ++
gfs2/fsck/initialize.c | 18 +-
gfs2/fsck/pass2.c | 8
gfs2/glocktop/glocktop.c| 6 +++---
gfs2/libgfs2/check_fs_ops.c | 18 +++---
gfs2/libgfs2/fs_ops.c | 38 +++--
gfs2/libgfs2/gfs2l.c| 2 +-
gfs2/libgfs2/lang.c | 10 ++
gfs2/libgfs2/libgfs2.h | 3 +--
tests/nukerg.c | 2 +-
12 files changed, 62 insertions(+), 66 deletions(-)
diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index e0ca7c71..bf9985ae 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -2139,11 +2139,10 @@ static void copy_quotas(struct lgfs2_sbd *sdp)
{
struct lgfs2_inum inum;
struct lgfs2_inode *oq_ip, *nq_ip;
- int err;
- err = lgfs2_lookupi(sdp->master_dir, "quota", 5, _ip);
- if (err) {
- fprintf(stderr, _("Couldn't lookup new quota file: %d\n"), err);
+ nq_ip = lgfs2_lookupi(sdp->master_dir, "quota", 5);
+ if (nq_ip == NULL) {
+ perror(_("Failed to look up new quota file"));
exit(1);
}
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 553b15a5..c779433c 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -925,7 +925,7 @@ static void read_superblock(int fd)
if (sbd.master_dir == NULL) {
sbd.md.riinode = NULL;
} else {
- lgfs2_lookupi(sbd.master_dir, "rindex", 6,
);
+ sbd.md.riinode = lgfs2_lookupi(sbd.master_dir,
"rindex", 6);
}
}
lgfs2_brelse(bh);
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 8e572523..47433eac 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -789,10 +789,9 @@ int ji_update(struct lgfs2_sbd *sdp)
return -1;
} else {
/* FIXME check snprintf return code */
- snprintf(journal_name, JOURNAL_NAME_SIZE,
-"journal%u", i);
- lgfs2_lookupi(sdp->md.jiinode, journal_name,
-strlen(journal_name), );
+ int len;
+ len = snprintf(journal_name, JOURNAL_NAME_SIZE,
"journal%u", i);
+ jip = lgfs2_lookupi(sdp->md.jiinode, journal_name, len);
sdp->md.journal[i] = jip;
}
}
@@ -884,7 +883,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
if (sdp->gfs1)
sdp->md.jiinode = lgfs2_inode_read(sdp,
sdp->sd_jindex_di.in_addr);
else
- lgfs2_lookupi(sdp->master_dir, "jindex", 6, >md.jiinode);
+ sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir, "jindex", 6);
if (!sdp->md.jiinode) {
int err;
@@ -906,7 +905,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
log_crit(_("Error %d rebuilding jindex\n"), err);
return err;
}
- lgfs2_lookupi(sdp->master_dir, "jindex", 6, >md.jiinode);
+ sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir, "jindex", 6);
}
/* check for irrelevant entries in jindex. Can't use check_dir because
@@ -938,8 +937,7 @@ int init_jindex(struct fsck_cx *cx, int allow_ji_rebuild)
"index: Cannot continue.\n"));
return error;
}
- lgfs2_lookupi(sdp->master_dir, "jindex", 6,
->md.jiinode);
+ sdp->md.jiinode = lgfs2_lookupi(sdp->master_dir,
"jindex", 6);
}
}
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index edb0b7fd..e8a8cb8b 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -597,7 +597,7 @@ static void lookup_per_node(struct fsck_cx *cx, int
allow_rebuild)
if (sdp->md.pinode)
return;
- lgfs2_lookupi(sdp->master_dir, "per_node", 8, >md.pinode);
+ sdp->md.pinode = lgfs2_lookupi(sdp->master_dir, "per_node", 8);
if (sdp->md.pinode)
return;
if (!allow_rebuild) {
@@ -618,7 +618,7 @@ static void lookup_per_node(struct fsck_cx *cx, int
allow_rebuild)
exit(FSCK_ERROR);
}
}
- lgfs2_lookupi(sdp->master_dir, "per_node", 8, >md.pinode);
+ sdp->md.pinode =
[Cluster-devel] [PATCH 3/6] libgfs2: Reorganise lgfs2_createi()
Move the lookup for an existing inode to lgfs2_createi itself and then
create the inode in __createi (renamed to do_createi) unconditionally.
Signed-off-by: Andrew Price
---
gfs2/libgfs2/fs_ops.c | 80 ---
1 file changed, 45 insertions(+), 35 deletions(-)
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index 5003c1ae..0df78e5a 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -1497,9 +1497,10 @@ int lgfs2_write_filemeta(struct lgfs2_inode *ip)
return 0;
}
-static struct lgfs2_inode *__createi(struct lgfs2_inode *dip, const char
*filename,
- unsigned int mode, uint32_t flags)
+static struct lgfs2_inode *do_createi(struct lgfs2_inode *dip, const char
*filename,
+ unsigned int mode, uint32_t flags)
{
+ struct lgfs2_inum parent = dip->i_num;
struct lgfs2_sbd *sdp = dip->i_sbd;
uint64_t bn;
struct lgfs2_inum inum;
@@ -1508,50 +1509,59 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode
*dip, const char *filena
int err = 0;
int is_dir;
- ip = lgfs2_lookupi(dip, filename, strlen(filename));
- if (!ip) {
- struct lgfs2_inum parent = dip->i_num;
-
- err = lgfs2_dinode_alloc(sdp, 1, );
- if (err != 0)
- return NULL;
-
- if (sdp->gfs1)
- inum.in_formal_ino = bn;
- else
- inum.in_formal_ino = sdp->md.next_inum++;
- inum.in_addr = bn;
-
- err = lgfs2_dir_add(dip, filename, strlen(filename), ,
IF2DT(mode));
- if (err)
- return NULL;
+ err = lgfs2_dinode_alloc(sdp, 1, );
+ if (err != 0)
+ return NULL;
- if (sdp->gfs1)
- is_dir = (IF2DT(mode) == GFS_FILE_DIR);
- else
- is_dir = S_ISDIR(mode);
- if (is_dir) {
- lgfs2_bmodified(dip->i_bh);
- dip->i_nlink++;
- }
+ if (sdp->gfs1)
+ inum.in_formal_ino = bn;
+ else
+ inum.in_formal_ino = sdp->md.next_inum++;
+ inum.in_addr = bn;
- err = __init_dinode(sdp, , , mode, flags, ,
sdp->gfs1);
- if (err != 0)
- return NULL;
+ err = lgfs2_dir_add(dip, filename, strlen(filename), ,
IF2DT(mode));
+ if (err)
+ return NULL;
- ip = lgfs2_inode_get(sdp, bh);
- if (ip == NULL)
- return NULL;
- lgfs2_bmodified(bh);
+ if (sdp->gfs1)
+ is_dir = (IF2DT(mode) == GFS_FILE_DIR);
+ else
+ is_dir = S_ISDIR(mode);
+ if (is_dir) {
+ lgfs2_bmodified(dip->i_bh);
+ dip->i_nlink++;
}
+ err = __init_dinode(sdp, , , mode, flags, , sdp->gfs1);
+ if (err != 0)
+ return NULL;
+
+ ip = lgfs2_inode_get(sdp, bh);
+ if (ip == NULL)
+ return NULL;
+ lgfs2_bmodified(bh);
ip->bh_owned = 1;
return ip;
}
+/**
+ * Create an inode and link it into a directory. If it already exists, return
+ * the existing inode. To create gfs1 inodes, dip->i_sbd->gfs1 must be set.
+ * @dip: The inode of the parent directory.
+ * @filename: The new inode's filename.
+ * @mode: The mode of the new inode.
+ * @flags: The flags of the new inode.
+ * Returns the new or existing inode on success or NULL on failure with errno
set.
+ */
struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char
*filename,
unsigned int mode, uint32_t flags)
{
- return __createi(dip, filename, mode, flags);
+ struct lgfs2_inode *ip = lgfs2_lookupi(dip, filename, strlen(filename));
+
+ if (ip != NULL) {
+ ip->bh_owned = 1;
+ return ip;
+ }
+ return do_createi(dip, filename, mode, flags);
}
/**
--
2.39.0
[Cluster-devel] [PATCH 2/6] libgfs2: Remove lgfs2_gfs_createi()
The one caller sets sdp->gfs1 and modifies the mode itself so we can use
sdp->gfs1 in __createi() and update the caller to use lgfs2_createi()
instead.
Signed-off-by: Andrew Price
---
gfs2/fsck/lost_n_found.c | 6 +-
gfs2/libgfs2/fs_ops.c| 19 ++-
gfs2/libgfs2/libgfs2.h | 3 ---
3 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index ca5ab89e..97fc16b0 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -109,11 +109,7 @@ void make_sure_lf_exists(struct fsck_cx *cx, struct
lgfs2_inode *ip)
if (sdp->gfs1)
sdp->md.next_inum = find_free_blk(sdp);
mode = (sdp->gfs1 ? DT2IF(GFS_FILE_DIR) : S_IFDIR) | 0700;
- if (sdp->gfs1)
- lf_dip = lgfs2_gfs_createi(sdp->md.rooti, "lost+found", mode,
0);
- else
- lf_dip = lgfs2_createi(sdp->md.rooti, "lost+found",
-S_IFDIR | 0700, 0);
+ lf_dip = lgfs2_createi(sdp->md.rooti, "lost+found", mode, 0);
if (lf_dip == NULL) {
log_crit(_("Error creating lost+found: %s\n"),
strerror(errno));
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index e72871ed..5003c1ae 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -1497,9 +1497,8 @@ int lgfs2_write_filemeta(struct lgfs2_inode *ip)
return 0;
}
-static struct lgfs2_inode *__createi(struct lgfs2_inode *dip,
- const char *filename, unsigned int mode,
- uint32_t flags, int if_gfs1)
+static struct lgfs2_inode *__createi(struct lgfs2_inode *dip, const char
*filename,
+ unsigned int mode, uint32_t flags)
{
struct lgfs2_sbd *sdp = dip->i_sbd;
uint64_t bn;
@@ -1517,7 +1516,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode
*dip,
if (err != 0)
return NULL;
- if (if_gfs1)
+ if (sdp->gfs1)
inum.in_formal_ino = bn;
else
inum.in_formal_ino = sdp->md.next_inum++;
@@ -1527,7 +1526,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode
*dip,
if (err)
return NULL;
- if (if_gfs1)
+ if (sdp->gfs1)
is_dir = (IF2DT(mode) == GFS_FILE_DIR);
else
is_dir = S_ISDIR(mode);
@@ -1536,7 +1535,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode
*dip,
dip->i_nlink++;
}
- err = __init_dinode(sdp, , , mode, flags, ,
if_gfs1);
+ err = __init_dinode(sdp, , , mode, flags, ,
sdp->gfs1);
if (err != 0)
return NULL;
@@ -1552,13 +1551,7 @@ static struct lgfs2_inode *__createi(struct lgfs2_inode
*dip,
struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char
*filename,
unsigned int mode, uint32_t flags)
{
- return __createi(dip, filename, mode, flags, 0);
-}
-
-struct lgfs2_inode *lgfs2_gfs_createi(struct lgfs2_inode *dip, const char
*filename,
- unsigned int mode, uint32_t flags)
-{
- return __createi(dip, filename, mode, flags, 1);
+ return __createi(dip, filename, mode, flags);
}
/**
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index bcbc5e47..69a7552f 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -515,9 +515,6 @@ extern int lgfs2_init_dinode(struct lgfs2_sbd *sdp, struct
lgfs2_buffer_head **b
unsigned int mode, uint32_t flags, struct lgfs2_inum
*parent);
extern struct lgfs2_inode *lgfs2_createi(struct lgfs2_inode *dip, const char
*filename,
unsigned int mode, uint32_t flags);
-extern struct lgfs2_inode *lgfs2_gfs_createi(struct lgfs2_inode *dip,
- const char *filename, unsigned int mode,
- uint32_t flags);
extern void lgfs2_dirent2_del(struct lgfs2_inode *dip, struct
lgfs2_buffer_head *bh,
struct gfs2_dirent *prev, struct gfs2_dirent *cur);
extern int lgfs2_dir_search(struct lgfs2_inode *dip, const char *filename, int
len,
--
2.39.0
Re: [Cluster-devel] [PATCH] gfs2: Fix uaf for qda in gfs2_quota_sync
Hello Edward, On Fri, Jan 27, 2023 at 6:12 AM wrote: > From: Edward Adam Davis > > [ 81.372851][ T5532] CPU: 1 PID: 5532 Comm: syz-executor.0 Not tainted > 6.2.0-rc1-syzkaller-dirty #0 > [ 81.382080][ T5532] Hardware name: Google Google Compute Engine/Google > Compute Engine, BIOS Google 01/12/2023 > [ 81.392343][ T5532] Call Trace: > [ 81.395654][ T5532] > [ 81.398603][ T5532] dump_stack_lvl+0x1b1/0x290 > [ 81.418421][ T5532] gfs2_assert_warn_i+0x19a/0x2e0 > [ 81.423480][ T5532] gfs2_quota_cleanup+0x4c6/0x6b0 > [ 81.428611][ T5532] gfs2_make_fs_ro+0x517/0x610 > [ 81.457802][ T5532] gfs2_withdraw+0x609/0x1540 > [ 81.481452][ T5532] gfs2_inode_refresh+0xb2d/0xf60 > [ 81.506658][ T5532] gfs2_instantiate+0x15e/0x220 > [ 81.511504][ T5532] gfs2_glock_wait+0x1d9/0x2a0 > [ 81.516352][ T5532] do_sync+0x485/0xc80 > [ 81.554943][ T5532] gfs2_quota_sync+0x3da/0x8b0 > [ 81.559738][ T5532] gfs2_sync_fs+0x49/0xb0 > [ 81.564063][ T5532] sync_filesystem+0xe8/0x220 > [ 81.568740][ T5532] generic_shutdown_super+0x6b/0x310 > [ 81.574112][ T5532] kill_block_super+0x79/0xd0 > [ 81.578779][ T5532] deactivate_locked_super+0xa7/0xf0 > [ 81.584064][ T5532] cleanup_mnt+0x494/0x520 > [ 81.593753][ T5532] task_work_run+0x243/0x300 > [ 81.608837][ T5532] exit_to_user_mode_loop+0x124/0x150 > [ 81.614232][ T5532] exit_to_user_mode_prepare+0xb2/0x140 > [ 81.619820][ T5532] syscall_exit_to_user_mode+0x26/0x60 > [ 81.625287][ T5532] do_syscall_64+0x49/0xb0 > [ 81.629710][ T5532] entry_SYSCALL_64_after_hwframe+0x63/0xcd > [ 81.636292][ T5532] RIP: 0033:0x7efdd688d517 > [ 81.640728][ T5532] Code: ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f > 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 > <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 > [ 81.660550][ T5532] RSP: 002b:7fff34520ce8 EFLAGS: 0246 ORIG_RAX: > 00a6 > [ 81.669413][ T5532] RAX: RBX: RCX: > 7efdd688d517 > [ 81.677403][ T5532] RDX: 7fff34520db9 RSI: 000a RDI: > 7fff34520db0 > [ 81.685388][ T5532] RBP: 7fff34520db0 R08: R09: > 7fff34520b80 > [ 81.695973][ T5532] R10: 55ca38b3 R11: 0246 R12: > 7efdd68e6b24 > [ 81.704152][ T5532] R13: 7fff34521e70 R14: 55ca3810 R15: > 7fff34521eb0 > [ 81.712868][ T5532] > > The function "gfs2_quota_cleanup()" may be called in the function "do_sync()", > This will cause the qda obtained in the function "qd_check_sync" to be > released, resulting in the occurrence of uaf. > In order to avoid this uaf, we can increase the judgment of > "sdp->sd_quota_bitmap" released in the function > "gfs2_quota_cleanup" to confirm that "sdp->sd_quota_list" has been released. I can see that there is a problem in the gfs2 quota code, but this unfortunately doesn't look like an acceptable fix. A better approach would be to use proper reference counting for gfs2_quota_data objects. In this case, gfs2_quota_sync() is still holding a reference, so the underlying object shouldn't be freed. Fixing this properly will require more than a handful of lines. > Link: https://lore.kernel.org/all/2b5e2405f14e8...@google.com > Reported-and-tested-by: syzbot+3f6a670108ce43356...@syzkaller.appspotmail.com > Signed-off-by: Edward Adam Davis > --- > fs/gfs2/quota.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c > index 1ed1722..4cf66bd 100644 > --- a/fs/gfs2/quota.c > +++ b/fs/gfs2/quota.c > @@ -1321,6 +1321,9 @@ int gfs2_quota_sync(struct super_block *sb, int type) > qda[x]->qd_sync_gen = > sdp->sd_quota_sync_gen; > > + if (!sdp->sd_quota_bitmap) > + break; > + > for (x = 0; x < num_qd; x++) > qd_unlock(qda[x]); > } > -- > 2.39.0 > Thanks, Andreas
[Cluster-devel] [PATCH] gfs2: Fix uaf for qda in gfs2_quota_sync
From: Edward Adam Davis [ 81.372851][ T5532] CPU: 1 PID: 5532 Comm: syz-executor.0 Not tainted 6.2.0-rc1-syzkaller-dirty #0 [ 81.382080][ T5532] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023 [ 81.392343][ T5532] Call Trace: [ 81.395654][ T5532] [ 81.398603][ T5532] dump_stack_lvl+0x1b1/0x290 [ 81.418421][ T5532] gfs2_assert_warn_i+0x19a/0x2e0 [ 81.423480][ T5532] gfs2_quota_cleanup+0x4c6/0x6b0 [ 81.428611][ T5532] gfs2_make_fs_ro+0x517/0x610 [ 81.457802][ T5532] gfs2_withdraw+0x609/0x1540 [ 81.481452][ T5532] gfs2_inode_refresh+0xb2d/0xf60 [ 81.506658][ T5532] gfs2_instantiate+0x15e/0x220 [ 81.511504][ T5532] gfs2_glock_wait+0x1d9/0x2a0 [ 81.516352][ T5532] do_sync+0x485/0xc80 [ 81.554943][ T5532] gfs2_quota_sync+0x3da/0x8b0 [ 81.559738][ T5532] gfs2_sync_fs+0x49/0xb0 [ 81.564063][ T5532] sync_filesystem+0xe8/0x220 [ 81.568740][ T5532] generic_shutdown_super+0x6b/0x310 [ 81.574112][ T5532] kill_block_super+0x79/0xd0 [ 81.578779][ T5532] deactivate_locked_super+0xa7/0xf0 [ 81.584064][ T5532] cleanup_mnt+0x494/0x520 [ 81.593753][ T5532] task_work_run+0x243/0x300 [ 81.608837][ T5532] exit_to_user_mode_loop+0x124/0x150 [ 81.614232][ T5532] exit_to_user_mode_prepare+0xb2/0x140 [ 81.619820][ T5532] syscall_exit_to_user_mode+0x26/0x60 [ 81.625287][ T5532] do_syscall_64+0x49/0xb0 [ 81.629710][ T5532] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 81.636292][ T5532] RIP: 0033:0x7efdd688d517 [ 81.640728][ T5532] Code: ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 31 f6 e9 09 00 00 00 66 0f 1f 84 00 00 00 00 00 b8 a6 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48 [ 81.660550][ T5532] RSP: 002b:7fff34520ce8 EFLAGS: 0246 ORIG_RAX: 00a6 [ 81.669413][ T5532] RAX: RBX: RCX: 7efdd688d517 [ 81.677403][ T5532] RDX: 7fff34520db9 RSI: 000a RDI: 7fff34520db0 [ 81.685388][ T5532] RBP: 7fff34520db0 R08: R09: 7fff34520b80 [ 81.695973][ T5532] R10: 55ca38b3 R11: 0246 R12: 7efdd68e6b24 [ 81.704152][ T5532] R13: 7fff34521e70 R14: 55ca3810 R15: 7fff34521eb0 [ 81.712868][ T5532] The function "gfs2_quota_cleanup()" may be called in the function "do_sync()", This will cause the qda obtained in the function "qd_check_sync" to be released, resulting in the occurrence of uaf. In order to avoid this uaf, we can increase the judgment of "sdp->sd_quota_bitmap" released in the function "gfs2_quota_cleanup" to confirm that "sdp->sd_quota_list" has been released. Link: https://lore.kernel.org/all/2b5e2405f14e8...@google.com Reported-and-tested-by: syzbot+3f6a670108ce43356...@syzkaller.appspotmail.com Signed-off-by: Edward Adam Davis --- fs/gfs2/quota.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c index 1ed1722..4cf66bd 100644 --- a/fs/gfs2/quota.c +++ b/fs/gfs2/quota.c @@ -1321,6 +1321,9 @@ int gfs2_quota_sync(struct super_block *sb, int type) qda[x]->qd_sync_gen = sdp->sd_quota_sync_gen; + if (!sdp->sd_quota_bitmap) + break; + for (x = 0; x < num_qd; x++) qd_unlock(qda[x]); } -- 2.39.0
