Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758539Ab3GZKYV (ORCPT ); Fri, 26 Jul 2013 06:24:21 -0400 Received: from mail9.hitachi.co.jp ([133.145.228.44]:41251 "EHLO mail9.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758528Ab3GZKYO (ORCPT ); Fri, 26 Jul 2013 06:24:14 -0400 Message-ID: <51F24E4A.3080803@hitachi.com> Date: Fri, 26 Jul 2013 19:24:10 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 5.2; rv:13.0) Gecko/20120614 Thunderbird/13.0.1 MIME-Version: 1.0 To: Oleg Nesterov Cc: Steven Rostedt , Linus Torvalds , Al Viro , Greg Kroah-Hartman , Alexander Z Lam , Arnaldo Carvalho de Melo , David Sharp , Frederic Weisbecker , Ingo Molnar , Peter Zijlstra , Srikar Dronamraju , Vaibhav Nagarnaik , "zhangwei(Jovi)" , linux-kernel@vger.kernel.org Subject: Re: Re: PATCH? debugfs_remove_recursive() must not rely on list_empty(d_subdirs) References: <20130723205854.GA9036@redhat.com> <20130724184640.GA21322@redhat.com> <20130725192742.GA14060@redhat.com> <20130725200423.GA22274@redhat.com> In-Reply-To: <20130725200423.GA22274@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2270 Lines: 88 (2013/07/26 5:04), Oleg Nesterov wrote: > On 07/25, Oleg Nesterov wrote: >> >> To simplify the review, this is how it looks with the patch applied: > > v2. We can use simply use list_for_each_entry_safe() and > list_next_entry() should be calles under ->i_mutex. Although > debugfs_remove_recursive() can race with itself anyway, but > still. > > And the code looks much simpler. But I do not know what did > I miss. > > Oleg. > > void debugfs_remove_recursive(struct dentry *dentry) > { > struct dentry *child, *next, *parent; > > if (IS_ERR_OR_NULL(dentry)) > return; > > parent = dentry->d_parent; > if (!parent || !parent->d_inode) > return; > > parent = dentry; > down: > mutex_lock(&parent->d_inode->i_mutex); > list_for_each_entry_safe(child, next, &parent->d_subdirs, d_u.d_child) { Perhaps, you can use list_for_each_entry_safe_continue() here, as below. parent = dentry; down: child = list_first_entry_or_null(&parent->d_subdirs, typeof(*child), d_u.d_child); mutex_lock(&parent->d_inode->i_mutex); restart: list_for_each_entry_safe_continue(child, next, &parent->d_subdirs, d_u.d_child) { > if (!debugfs_positive(child)) > continue; > > /* XXX: simple_empty(child) instead ? */ > if (!list_empty(&child->d_subdirs)) { > mutex_unlock(&parent->d_inode->i_mutex); > parent = child; > goto down; > } > up: > __debugfs_remove(child, parent); > } Then, you can avoid jumping into the loop, just restart it from parent as below. mutex_unlock(&parent->d_inode->i_mutex); child = parent; parent = parent->d_parent; mutex_lock(&parent->d_inode->i_mutex); __debugfs_remove(child, parent); if (child != dentry) goto restart; > mutex_unlock(&parent->d_inode->i_mutex); > simple_release_fs(&debugfs_mount, &debugfs_mount_count); > } It's just an idea, which came up to my mind. :) Thank you, -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/