From d04a1929b3adb776173b02e0f6b82d396046dd14 Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Fri, 12 Nov 2021 11:27:42 -0500 Subject: [PATCH] LU-15127 llite: Remove path from discard_warn It is unfortunately not safe to get the path from inside dirty page discard warn. It results in us getting and then putting a bunch of dentries, and if 'dget' we do on our file is the last reference on it, we deadlock like this: ptlrpc_check_set brw_interpret osc_extent_finish osc_ap_completion cl_page_completion vvp_page_completion_write ll_dirty_page_discard_warn dput dentry_kill __dentry_kill evict ll_delete_inode cl_sync_file_range cl_io_loop cl_io_start lov_io_call cl_io_start osc_io_fsync_start osc_cache_writeback_range osc_cache_wait_range osc_extent_wait ll_delete_inode is calling back in to *this file*, which we are already working on, so this thread ends up waiting for itself. This is particularly common if the discard warn is racing with an unmount, which will be destroying all the inodes (not deleting them - just removing them from the local VFS). There is no way to safely get the path from this location. If we are deeply committed to the functionality, it would be possible to rewrite osc_extent_finish + brw_interpret so they could attempt path lookup *after* the extent has been completed. This patch fixes the deadlock, any rewrite is left for later. Signed-off-by: Patrick Farrell Change-Id: I537fd0d2e110c180a1369a9a3b1a644e613b18e4 --- lustre/llite/llite_lib.c | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index 87ccf1b..a117762 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -3326,47 +3326,16 @@ int ll_get_obd_name(struct inode *inode, unsigned int cmd, unsigned long arg) RETURN(0); } -static char* ll_d_path(struct dentry *dentry, char *buf, int bufsize) -{ - char *path = NULL; - - struct path p; - - p.dentry = dentry; - p.mnt = current->fs->root.mnt; - path_get(&p); - path = d_path(&p, buf, bufsize); - path_put(&p); - return path; -} - void ll_dirty_page_discard_warn(struct page *page, int ioret) { - char *buf, *path = NULL; - struct dentry *dentry = NULL; struct inode *inode = page->mapping->host; - /* this can be called inside spin lock so use GFP_ATOMIC. */ - buf = (char *)__get_free_page(GFP_ATOMIC); - if (buf != NULL) { - dentry = d_find_alias(page->mapping->host); - if (dentry != NULL) - path = ll_d_path(dentry, buf, PAGE_SIZE); - } - /* The below message is checked in recovery-small.sh test_24b */ CDEBUG(D_WARNING, - "%s: dirty page discard: %s/fid: "DFID"/%s may get corrupted " + "%s: dirty page discard: %s/fid: "DFID" may get corrupted " "(rc %d)\n", ll_i2sbi(inode)->ll_fsname, s2lsi(page->mapping->host->i_sb)->lsi_lmd->lmd_dev, - PFID(ll_inode2fid(inode)), - (path && !IS_ERR(path)) ? path : "", ioret); - - if (dentry != NULL) - dput(dentry); - - if (buf != NULL) - free_page((unsigned long)buf); + PFID(ll_inode2fid(inode)), ioret); } ssize_t ll_copy_user_md(const struct lov_user_md __user *md, -- 1.8.3.1