Re: [PATCH 1/2] configfs: Silence lockdep on mkdir() and rmdir()

[Louis Rilling <Louis.Rilling@kerlabs.com>]

[-- Attachment #1: Type: text/plain, Size: 3723 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]