Messages in this thread | | | Date | Wed, 28 Jan 2009 11:38:43 +0100 | From | Louis Rilling <> | Subject | Re: [PATCH 1/2] configfs: Silence lockdep on mkdir() and rmdir() |
| |
On 27/01/09 19:55 -0800, Joel Becker wrote: > On Thu, Dec 18, 2008 at 07:00:17PM +0100, Louis Rilling wrote:
[...]
> > > #define CONFIGFS_ROOT 0x0001 > > diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c > > index 8e93341..f21be74 100644 > > --- a/fs/configfs/dir.c > > +++ b/fs/configfs/dir.c > > @@ -94,6 +94,9 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent * pare > > INIT_LIST_HEAD(&sd->s_links); > > INIT_LIST_HEAD(&sd->s_children); > > sd->s_element = element; > > +#ifdef CONFIG_LOCKDEP > > + sd->s_depth = -1; > > +#endif > > spin_lock(&configfs_dirent_lock); > > if (parent_sd->s_type & CONFIGFS_USET_DROPPING) { > > spin_unlock(&configfs_dirent_lock); > > @@ -176,6 +179,17 @@ static int init_symlink(struct inode * inode) > > return 0; > > } > > > > +#ifdef CONFIG_LOCKDEP > > +static void configfs_set_dir_dirent_depth(struct configfs_dirent *parent_sd, > > + struct configfs_dirent *sd) > > +{ > > + int parent_depth = parent_sd->s_depth; > > + > > + if (parent_depth >= 0) > > + sd->s_depth = parent_depth + 1; > > +} > > +#endif > > + > > static int create_dir(struct config_item * k, struct dentry * p, > > struct dentry * d) > > { > > @@ -187,6 +201,9 @@ static int create_dir(struct config_item * k, struct dentry * p, > > error = configfs_make_dirent(p->d_fsdata, d, k, mode, > > CONFIGFS_DIR | CONFIGFS_USET_CREATING); > > if (!error) { > > +#ifdef CONFIG_LOCKDEP > > + configfs_set_dir_dirent_depth(p->d_fsdata, d->d_fsdata); > > +#endif > > error = configfs_create(d, mode, init_dir); > > if (!error) { > > inc_nlink(p->d_inode); > > Can you change this to provide non-lockdep versions of > functions? We don't want "#ifdef CONFIG_LOCKDEP" everywhere. What we > want is the code to call functions unconditionally, and the functions to > do nothing if lockdep is not enabled. Like: > > #ifdef CONFIG_LOCKDEP > static inline void configfs_init_dir_dirent_depth(dirent) > { > dirent->s_depth = -1; > } > > static void configfs_set_dir_dirent_depth(struct configfs_dirent *parent_sd, > struct configfs_dirent *sd) > { > int parent_depth = parent_sd->s_depth; > > if (parent_depth >= 0) > sd->s_depth = parent_depth + 1; > } > #else > static inline void configfs_init_dir_dirent_depth(dirent) > { > } > > static void configfs_set_dir_dirent_depth(struct configfs_dirent *parent_sd, > struct configfs_dirent *sd) > { > } > #endif > > This makes the callsites much nicer to read: > > @@ -94,6 +94,9 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent * pare > INIT_LIST_HEAD(&sd->s_links); > INIT_LIST_HEAD(&sd->s_children); > sd->s_element = element; > + configfs_init_dir_dirent_depth(sd); > spin_lock(&configfs_dirent_lock); > if (parent_sd->s_type & CONFIGFS_USET_DROPPING) { > spin_unlock(&configfs_dirent_lock); > @@ -187,6 +201,7 @@ static int create_dir(struct config_item * k, struct dentry * p, > error = configfs_make_dirent(p->d_fsdata, d, k, mode, > CONFIGFS_DIR | CONFIGFS_USET_CREATING); > if (!error) { > + configfs_set_dir_dirent_depth(p->d_fsdata, d->d_fsdata); > error = configfs_create(d, mode, init_dir); > if (!error) { > inc_nlink(p->d_inode);
Sure, that's why I said that this could be cleaner ;)
> > Otherwise, this patch seems pretty straightforward.
Cleaner patch coming.
Thanks,
Louis
-- Dr Louis Rilling Kerlabs Skype: louis.rilling Batiment Germanium Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes http://www.kerlabs.com/ 35700 Rennes [unhandled content-type:application/pgp-signature] | |