On Wed, Mar 6, 2019 at 10:15 PM Josef Bacik <jo...@toxicpanda.com> wrote: > > When Filipe added the recursive directory logging stuff he specifically > didn't take the directory i_mutex for the children directories that we > need to log because of lockdep. This is generally fine, but can lead to > this WARN_ON() tripping if we happen to run delayed deletion's in > between our first search and our second search of dir_item/dir_indexes > for this directory. We expect this to happen, so the WARN_ON() isn't > necessary. Drop the WARN_ON() and add a comment so we know why this > case can happen. > > Signed-off-by: Josef Bacik <jo...@toxicpanda.com>
Reviewed-by: Filipe Manana <fdman...@suse.com> Looks good, thanks! > --- > fs/btrfs/tree-log.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index f06454a55e00..bccb62c1c587 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -3578,9 +3578,16 @@ static noinline int log_dir_items(struct > btrfs_trans_handle *trans, > } > btrfs_release_path(path); > > - /* find the first key from this transaction again */ > + /* > + * Find the first key from this transaction again. See the note for > + * log_new_dir_dentries, if we're logging a directory recursively we > + * won't be holding its i_mutex, which means we can modify the > directory > + * while we're logging it. If we remove an entry between our first > + * search and this search we'll not find the key again and can just > + * bail. > + */ > ret = btrfs_search_slot(NULL, root, &min_key, path, 0, 0); > - if (WARN_ON(ret != 0)) > + if (ret != 0) > goto done; > > /* > -- > 2.14.3 > -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”
