lkml.org 
[lkml]   [2009]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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]
\
 
 \ /
  Last update: 2009-01-28 11:41    [W:0.071 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site