Re: [PATCH v3] fs/locks: print full locks information
在 2021/3/9 21:37, Jeff Layton 写道: On Thu, 2021-02-25 at 22:58 -0500, Luo Longjun wrote: Commit fd7732e033e3 ("fs/locks: create a tree of dependent requests.") has put blocked locks into a tree. So, with a for loop, we can't check all locks information. To solve this problem, we should traverse the tree. Signed-off-by: Luo Longjun --- fs/locks.c | 65 ++ 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 99ca97e81b7a..ecaecd1f1b58 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2828,7 +2828,7 @@ struct locks_iterator { }; static void lock_get_status(struct seq_file *f, struct file_lock *fl, - loff_t id, char *pfx) + loff_t id, char *pfx, int repeat) { struct inode *inode = NULL; unsigned int fl_pid; @@ -2844,7 +2844,11 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, if (fl->fl_file != NULL) inode = locks_inode(fl->fl_file); - seq_printf(f, "%lld:%s ", id, pfx); + seq_printf(f, "%lld: ", id); + + if (repeat) + seq_printf(f, "%*s", repeat - 1 + (int)strlen(pfx), pfx); Shouldn't that be "%.*s" ? Also, isn't this likely to end up walking past the end of "pfx" (or even ending up at an address before the buffer)? You have this below: lock_get_status(f, fl, *id, "", 0); ...so the "length" value you're passing into the format there is going to be -1. It also seems like if you get a large "level" value in locks_show, then you'll end up with a length that is much longer than the actual string. In my understanding, the difference of "%*s" and "%.*s" is that, "%*s" specifies the minimal filed width while "%.*s" specifies the precision of the string. Here, I use "%*s", because I want to print locks information in the follwing format: 2: FLOCK ADVISORY WRITE 110 00:02:493 0 EOF 2: -> FLOCK ADVISORY WRITE 111 00:02:493 0 EOF 2: -> FLOCK ADVISORY WRITE 112 00:02:493 0 EOF 2: -> FLOCK ADVISORY WRITE 113 00:02:493 0 EOF 2: -> FLOCK ADVISORY WRITE 114 00:02:493 0 EOF And also, there is another way to show there information, in the format like: 60: FLOCK ADVISORY WRITE 23350 08:02:4456514 0 EOF 60: -> FLOCK ADVISORY WRITE 23356 08:02:4456514 0 EOF 60: -> FLOCK ADVISORY WRITE 24217 08:02:4456514 0 EOF 60: -> FLOCK ADVISORY WRITE 24239 08:02:4456514 0 EOF I think both formats are acceptable, but the first format shows competition relationships between these locks. In the following code: lock_get_status(f, fl, *id, "", 0); repeat is 0, and in the function: + if (repeat) + seq_printf(f, "%*s", repeat - 1 + (int)strlen(pfx), pfx); The if branch will not take effect, so it could not be -1. + if (IS_POSIX(fl)) { if (fl->fl_flags & FL_ACCESS) seq_puts(f, "ACCESS"); @@ -2906,21 +2910,64 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, } } +static struct file_lock *get_next_blocked_member(struct file_lock *node) +{ + struct file_lock *tmp; + + /* NULL node or root node */ + if (node == NULL || node->fl_blocker == NULL) + return NULL; + + /* Next member in the linked list could be itself */ + tmp = list_next_entry(node, fl_blocked_member); + if (list_entry_is_head(tmp, &node->fl_blocker->fl_blocked_requests, fl_blocked_member) + || tmp == node) { + return NULL; + } + + return tmp; +} + static int locks_show(struct seq_file *f, void *v) { struct locks_iterator *iter = f->private; - struct file_lock *fl, *bfl; + struct file_lock *cur, *tmp; struct pid_namespace *proc_pidns = proc_pid_ns(file_inode(f->file)->i_sb); + int level = 0; - fl = hlist_entry(v, struct file_lock, fl_link); + cur = hlist_entry(v, struct file_lock, fl_link); - if (locks_translate_pid(fl, proc_pidns) == 0) + if (locks_translate_pid(cur, proc_pidns) == 0) return 0; - lock_get_status(f, fl, iter->li_pos, ""); + /* View this crossed linked list as a binary tree, the first member of fl_blocked_requests +* is the left child of current node, the next silibing in fl_blocked_member is the +* right child, we can alse get the parent of current node from fl_blocker, so this +* question becomes traversal of a binary tree +*/ + while (cur != NULL) { + if (level) + lock_get_status(f, cur, iter->li_pos, "-&
[PATCH v3] fs/locks: print full locks information
Commit fd7732e033e3 ("fs/locks: create a tree of dependent requests.") has put blocked locks into a tree. So, with a for loop, we can't check all locks information. To solve this problem, we should traverse the tree. Signed-off-by: Luo Longjun --- fs/locks.c | 65 ++ 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 99ca97e81b7a..ecaecd1f1b58 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2828,7 +2828,7 @@ struct locks_iterator { }; static void lock_get_status(struct seq_file *f, struct file_lock *fl, - loff_t id, char *pfx) + loff_t id, char *pfx, int repeat) { struct inode *inode = NULL; unsigned int fl_pid; @@ -2844,7 +2844,11 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, if (fl->fl_file != NULL) inode = locks_inode(fl->fl_file); - seq_printf(f, "%lld:%s ", id, pfx); + seq_printf(f, "%lld: ", id); + + if (repeat) + seq_printf(f, "%*s", repeat - 1 + (int)strlen(pfx), pfx); + if (IS_POSIX(fl)) { if (fl->fl_flags & FL_ACCESS) seq_puts(f, "ACCESS"); @@ -2906,21 +2910,64 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, } } +static struct file_lock *get_next_blocked_member(struct file_lock *node) +{ + struct file_lock *tmp; + + /* NULL node or root node */ + if (node == NULL || node->fl_blocker == NULL) + return NULL; + + /* Next member in the linked list could be itself */ + tmp = list_next_entry(node, fl_blocked_member); + if (list_entry_is_head(tmp, &node->fl_blocker->fl_blocked_requests, fl_blocked_member) + || tmp == node) { + return NULL; + } + + return tmp; +} + static int locks_show(struct seq_file *f, void *v) { struct locks_iterator *iter = f->private; - struct file_lock *fl, *bfl; + struct file_lock *cur, *tmp; struct pid_namespace *proc_pidns = proc_pid_ns(file_inode(f->file)->i_sb); + int level = 0; - fl = hlist_entry(v, struct file_lock, fl_link); + cur = hlist_entry(v, struct file_lock, fl_link); - if (locks_translate_pid(fl, proc_pidns) == 0) + if (locks_translate_pid(cur, proc_pidns) == 0) return 0; - lock_get_status(f, fl, iter->li_pos, ""); + /* View this crossed linked list as a binary tree, the first member of fl_blocked_requests +* is the left child of current node, the next silibing in fl_blocked_member is the +* right child, we can alse get the parent of current node from fl_blocker, so this +* question becomes traversal of a binary tree +*/ + while (cur != NULL) { + if (level) + lock_get_status(f, cur, iter->li_pos, "-> ", level); + else + lock_get_status(f, cur, iter->li_pos, "", level); - list_for_each_entry(bfl, &fl->fl_blocked_requests, fl_blocked_member) - lock_get_status(f, bfl, iter->li_pos, " ->"); + if (!list_empty(&cur->fl_blocked_requests)) { + /* Turn left */ + cur = list_first_entry_or_null(&cur->fl_blocked_requests, + struct file_lock, fl_blocked_member); + level++; + } else { + /* Turn right */ + tmp = get_next_blocked_member(cur); + /* Fall back to parent node */ + while (tmp == NULL && cur->fl_blocker != NULL) { + cur = cur->fl_blocker; + level--; + tmp = get_next_blocked_member(cur); + } + cur = tmp; + } + } return 0; } @@ -2941,7 +2988,7 @@ static void __show_fd_locks(struct seq_file *f, (*id)++; seq_puts(f, "lock:\t"); - lock_get_status(f, fl, *id, ""); + lock_get_status(f, fl, *id, "", 0); } } -- 2.17.1
[PATCH v2 02/24] fs/locks: print full locks information
Commit fd7732e033e3 ("fs/locks: create a tree of dependent requests.") has put blocked locks into a tree. So, with a for loop, we can't check all locks information. To solve this problem, we should traverse the tree by non-recursion DFS. Signed-off-by: Luo Longjun --- fs/locks.c | 75 -- 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 99ca97e81b7a..fdf240626777 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2827,8 +2827,14 @@ struct locks_iterator { loff_t li_pos; }; +struct locks_traverse_list { + struct list_head head; + struct file_lock *lock; + int level; +}; + static void lock_get_status(struct seq_file *f, struct file_lock *fl, - loff_t id, char *pfx) + loff_t id, char *pfx, int repeat) { struct inode *inode = NULL; unsigned int fl_pid; @@ -2844,7 +2850,11 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, if (fl->fl_file != NULL) inode = locks_inode(fl->fl_file); - seq_printf(f, "%lld:%s ", id, pfx); + seq_printf(f, "%lld: ", id); + + if (repeat) + seq_printf(f, "%*s", repeat - 1 + strlen(pfx), pfx); + if (IS_POSIX(fl)) { if (fl->fl_flags & FL_ACCESS) seq_puts(f, "ACCESS"); @@ -2912,17 +2922,66 @@ static int locks_show(struct seq_file *f, void *v) struct file_lock *fl, *bfl; struct pid_namespace *proc_pidns = proc_pid_ns(file_inode(f->file)->i_sb); + struct list_head root; + struct list_head *tail = &root; + struct list_head *pos, *tmp; + struct locks_traverse_list *node, *node_child; + + int ret = 0; + fl = hlist_entry(v, struct file_lock, fl_link); if (locks_translate_pid(fl, proc_pidns) == 0) - return 0; + return ret; + + INIT_LIST_HEAD(&root); - lock_get_status(f, fl, iter->li_pos, ""); + node = kmalloc(sizeof(struct locks_traverse_list), GFP_KERNEL); + if (!node) { + ret = -ENOMEM; + goto out; + } - list_for_each_entry(bfl, &fl->fl_blocked_requests, fl_blocked_member) - lock_get_status(f, bfl, iter->li_pos, " ->"); + node->level = 0; + node->lock = fl; + list_add(&node->head, tail); + tail = &node->head; - return 0; + while (tail != &root) { + node = list_entry(tail, struct locks_traverse_list, head); + if (!node->level) + lock_get_status(f, node->lock, iter->li_pos, "", node->level); + else + lock_get_status(f, node->lock, iter->li_pos, "-> ", node->level); + + tmp = tail->prev; + list_del(tail); + tail = tmp; + + list_for_each_entry_reverse(bfl, &node->lock->fl_blocked_requests, + fl_blocked_member) { + node_child = kmalloc(sizeof(struct locks_traverse_list), GFP_KERNEL); + if (!node_child) { + ret = -ENOMEM; + goto out; + } + + node_child->level = node->level + 1; + node_child->lock = bfl; + list_add(&node_child->head, tail); + tail = &node_child->head; + } + kfree(node); + } + +out: + list_for_each_safe(pos, tmp, &root) { + node = list_entry(pos, struct locks_traverse_list, head); + list_del(pos); + if (!node) + kfree(node); + } + return ret; } static void __show_fd_locks(struct seq_file *f, @@ -2941,7 +3000,7 @@ static void __show_fd_locks(struct seq_file *f, (*id)++; seq_puts(f, "lock:\t"); - lock_get_status(f, fl, *id, ""); + lock_get_status(f, fl, *id, "", 0); } } -- 2.17.1
[PATCH] fs/locks: print full locks information
Commit fd7732e033e3 ("fs/locks: create a tree of dependent requests.") has put blocked locks into a tree. So, with a for loop, we can't check all locks information. To solve this problem, we should traverse the tree by DFS. Signed-off-by: Luo Longjun --- fs/locks.c | 30 +- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index 99ca97e81b7a..1f7b6683ed54 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2828,9 +2828,10 @@ struct locks_iterator { }; static void lock_get_status(struct seq_file *f, struct file_lock *fl, - loff_t id, char *pfx) + loff_t id, char *pfx, int repeat) { struct inode *inode = NULL; + int i; unsigned int fl_pid; struct pid_namespace *proc_pidns = proc_pid_ns(file_inode(f->file)->i_sb); @@ -2844,7 +2845,13 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, if (fl->fl_file != NULL) inode = locks_inode(fl->fl_file); - seq_printf(f, "%lld:%s ", id, pfx); + seq_printf(f, "%lld: ", id); + for (i = 1; i < repeat; i++) + seq_puts(f, " "); + + if (repeat) + seq_printf(f, "%s", pfx); + if (IS_POSIX(fl)) { if (fl->fl_flags & FL_ACCESS) seq_puts(f, "ACCESS"); @@ -2906,6 +2913,19 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, } } +static int __locks_show(struct seq_file *f, struct file_lock *fl, int level) +{ + struct locks_iterator *iter = f->private; + struct file_lock *bfl; + + lock_get_status(f, fl, iter->li_pos, "-> ", level); + + list_for_each_entry(bfl, &fl->fl_blocked_requests, fl_blocked_member) + __locks_show(f, bfl, level + 1); + + return 0; +} + static int locks_show(struct seq_file *f, void *v) { struct locks_iterator *iter = f->private; @@ -2917,10 +2937,10 @@ static int locks_show(struct seq_file *f, void *v) if (locks_translate_pid(fl, proc_pidns) == 0) return 0; - lock_get_status(f, fl, iter->li_pos, ""); + lock_get_status(f, fl, iter->li_pos, "", 0); list_for_each_entry(bfl, &fl->fl_blocked_requests, fl_blocked_member) - lock_get_status(f, bfl, iter->li_pos, " ->"); + __locks_show(f, bfl, 1); return 0; } @@ -2941,7 +2961,7 @@ static void __show_fd_locks(struct seq_file *f, (*id)++; seq_puts(f, "lock:\t"); - lock_get_status(f, fl, *id, ""); + lock_get_status(f, fl, *id, "", 0); } } -- 2.17.1