2023-10-11 21:39:38

by Seamus Connor

[permalink] [raw]
Subject: [PATCH] [WIP] configfs: improve item creation performance

As the size of a directory increases item creation slows down.
Optimizing access to s_children removes this bottleneck.

dirents are already pinned into the cache, there is no need to scan the
s_children list looking for duplicate Items. The configfs_dirent_exists
check is moved to a location where it is called only during subsystem
initialization.

d_lookup will only need to call configfs_lookup in the case where the
item in question is not pinned to dcache. The only items not pinned to
dcache are attributes. These are placed at the front of the s_children
list, whilst pinned items are inserted at the back. configfs_lookup
stops scanning when it encounters the first pinned entry in s_children.

The assumption of the above optimizations is that there will be few
attributes, but potentially many Items in a given directory.

On my machine, creating 40,000 Items in a single directory takes roughly
40 seconds. With this patch applied, that time drops down to around 130
ms.

Signed-off-by: Seamus Connor <[email protected]>
Cc: Joel Becker <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: [email protected]
---

Thanks for taking a look. I've done some testing of this patch and it
seems to work well for my use case (LIO). I would like to get feedback
from the maintainers and work this change upstream if possible.

Thanks!


fs/configfs/configfs_internal.h | 3 +--
fs/configfs/dir.c | 29 ++++++++++++++++++++---------
fs/configfs/inode.c | 24 ------------------------
3 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index c0395363eab9..9501c4106bda 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -55,6 +55,7 @@ struct configfs_dirent {
#define CONFIGFS_USET_IN_MKDIR 0x0200
#define CONFIGFS_USET_CREATING 0x0400
#define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
+#define CONFIGFS_IS_PINNED (CONFIGFS_ROOT | CONFIGFS_DIR | CONFIGFS_ITEM_LINK)

extern struct mutex configfs_symlink_mutex;
extern spinlock_t configfs_dirent_lock;
@@ -73,8 +74,6 @@ extern int configfs_make_dirent(struct configfs_dirent *, struct dentry *,
void *, umode_t, int, struct configfs_fragment *);
extern int configfs_dirent_is_ready(struct configfs_dirent *);

-extern void configfs_hash_and_remove(struct dentry * dir, const char * name);
-
extern const unsigned char * configfs_get_name(struct configfs_dirent *sd);
extern void configfs_drop_dentry(struct configfs_dirent *sd, struct dentry *parent);
extern int configfs_setattr(struct user_namespace *mnt_userns,
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index d1f9d2632202..e426c6a30ad6 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -182,6 +182,11 @@ struct configfs_fragment *get_fragment(struct configfs_fragment *frag)
return frag;
}

+static bool configfs_dirent_is_pinned(struct configfs_dirent *sd)
+{
+ return sd->s_type & CONFIGFS_IS_PINNED;
+}
+
/*
* Allocates a new configfs_dirent and links it to the parent configfs_dirent
*/
@@ -207,7 +212,10 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
return ERR_PTR(-ENOENT);
}
sd->s_frag = get_fragment(frag);
- list_add(&sd->s_sibling, &parent_sd->s_children);
+ if (configfs_dirent_is_pinned(sd))
+ list_add_tail(&sd->s_sibling, &parent_sd->s_children);
+ else
+ list_add(&sd->s_sibling, &parent_sd->s_children);
spin_unlock(&configfs_dirent_lock);

return sd;
@@ -220,10 +228,11 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
*
* called with parent inode's i_mutex held
*/
-static int configfs_dirent_exists(struct configfs_dirent *parent_sd,
- const unsigned char *new)
+static int configfs_dirent_exists(struct dentry *dentry)
{
- struct configfs_dirent * sd;
+ struct configfs_dirent *parent_sd = dentry->d_parent->d_fsdata;
+ const unsigned char *new = dentry->d_name.name;
+ struct configfs_dirent *sd;

list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
if (sd->s_element) {
@@ -289,10 +298,6 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry,

BUG_ON(!item);

- error = configfs_dirent_exists(p->d_fsdata, dentry->d_name.name);
- if (unlikely(error))
- return error;
-
error = configfs_make_dirent(p->d_fsdata, dentry, item, mode,
CONFIGFS_DIR | CONFIGFS_USET_CREATING,
frag);
@@ -449,6 +454,10 @@ static struct dentry * configfs_lookup(struct inode *dir,

spin_lock(&configfs_dirent_lock);
list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+
+ if (configfs_dirent_is_pinned(sd))
+ break;
+
if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
!strcmp(configfs_get_name(sd), dentry->d_name.name)) {
struct configfs_attribute *attr = sd->s_element;
@@ -1878,7 +1887,9 @@ int configfs_register_subsystem(struct configfs_subsystem *subsys)
if (dentry) {
d_add(dentry, NULL);

- err = configfs_attach_group(sd->s_element, &group->cg_item,
+ err = configfs_dirent_exists(dentry);
+ if (!err)
+ err = configfs_attach_group(sd->s_element, &group->cg_item,
dentry, frag);
if (err) {
BUG_ON(d_inode(dentry));
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index b601610e9907..15964e62329d 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -218,27 +218,3 @@ void configfs_drop_dentry(struct configfs_dirent * sd, struct dentry * parent)
}
}

-void configfs_hash_and_remove(struct dentry * dir, const char * name)
-{
- struct configfs_dirent * sd;
- struct configfs_dirent * parent_sd = dir->d_fsdata;
-
- if (d_really_is_negative(dir))
- /* no inode means this hasn't been made visible yet */
- return;
-
- inode_lock(d_inode(dir));
- list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
- if (!sd->s_element)
- continue;
- if (!strcmp(configfs_get_name(sd), name)) {
- spin_lock(&configfs_dirent_lock);
- list_del_init(&sd->s_sibling);
- spin_unlock(&configfs_dirent_lock);
- configfs_drop_dentry(sd, dir);
- configfs_put(sd);
- break;
- }
- }
- inode_unlock(d_inode(dir));
-}
--
2.37.0


2023-10-12 20:19:04

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH] [WIP] configfs: improve item creation performance

On Wed, Oct 11, 2023 at 02:39:19PM -0700, Seamus Connor wrote:
> On my machine, creating 40,000 Items in a single directory takes roughly
> 40 seconds. With this patch applied, that time drops down to around 130
> ms.

Nice.

> @@ -207,7 +212,10 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
> return ERR_PTR(-ENOENT);
> }
> sd->s_frag = get_fragment(frag);
> - list_add(&sd->s_sibling, &parent_sd->s_children);
> + if (configfs_dirent_is_pinned(sd))
> + list_add_tail(&sd->s_sibling, &parent_sd->s_children);
> + else
> + list_add(&sd->s_sibling, &parent_sd->s_children);
> spin_unlock(&configfs_dirent_lock);

This is subtle. Your patch description of course describes why we are
partitioning the items and attributes, but that will get lost into the
memory hole very quickly. Please add a comment.

> @@ -449,6 +454,10 @@ static struct dentry * configfs_lookup(struct inode *dir,
>
> spin_lock(&configfs_dirent_lock);
> list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
> +
> + if (configfs_dirent_is_pinned(sd))
> + break;
> +
> if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
> !strcmp(configfs_get_name(sd), dentry->d_name.name)) {
> struct configfs_attribute *attr = sd->s_element;

There's a lack of symmetry here. The pinned check is an inline
function, whereas the `CONFIGFS_NOT_PINNED` check is an open-coded
bitmask. Why not just:

```
if (sd->s_type & CONFIGFS_IS_PINNED)
break;
```

Plus, aren't the pinned/not-pinned checks redundant? Can't we avoid the
extra conditional?


```
spin_lock(&configfs_dirent_lock);
list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
- if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
- !strcmp(configfs_get_name(sd), dentry->d_name.name)) {
+ /*
+ * The dirents for config_items are pinned in the
+ * dcache, so configfs_lookup() should never be called
+ * for items. Thus, we're only looking up attributes.
+ *
+ * s_children is ordered so that attributes
+ * (CONFIGFS_NOT_PINNED) come before items (see
+ * configfs_new_dirent(). If we have reached a child item,
+ * we are done looking.
+ */
+ if (!(sd->s_type & CONFIGFS_NOT_PINNED))
+ break;
+
+ if (!strcmp(configfs_get_name(sd), dentry->d_name.name)) {
struct configfs_attribute *attr = sd->s_element;
umode_t mode = (attr->ca_mode & S_IALLUGO) | S_IFREG;
```

> -void configfs_hash_and_remove(struct dentry * dir, const char * name)
> -{
> - struct configfs_dirent * sd;
> - struct configfs_dirent * parent_sd = dir->d_fsdata;

Man, I thought we removed this years ago:
https://lkml.indiana.edu/hypermail/linux/kernel/0803.0/0905.html. No
idea why that patch didn't land.

Thanks,
Joel

--

Life's Little Instruction Book #222

"Think twice before burdening a friend with a secret."

http://www.jlbec.org/
[email protected]

2023-10-13 00:00:02

by Seamus Connor

[permalink] [raw]
Subject: Re: [PATCH] [WIP] configfs: improve item creation performance

>
> This is subtle. Your patch description of course describes why we are
> partitioning the items and attributes, but that will get lost into the
> memory hole very quickly. Please add a comment.
>

Will do. And thanks for taking a look at the patch.

> Plus, aren't the pinned/not-pinned checks redundant? Can't we avoid the
> extra conditional?
>
>
> ```
> spin_lock(&configfs_dirent_lock);
> list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
> - if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
> - !strcmp(configfs_get_name(sd), dentry->d_name.name)) {

Unfortunately they are not. CONFIGFS_IS_PINNED and CONFIGFS_NOT_PINNED
are asymmetrical. There is the readdir cursor (s_type == 0) that might
be anywhere in the list and should not be treated as a pinned item,
but should also not be treated as an unpinned attribute.

Is there any testing that you could recommend for this change? So far
I've gone through testing of our internal components that depend on
configfs, but I haven't done anything targeted.

Thanks,
Seamus

2023-10-13 21:12:52

by Seamus Connor

[permalink] [raw]
Subject: [PATCH v2] configfs: improve item creation performance

As the size of a directory increases item creation slows down.
Optimizing access to s_children removes this bottleneck.

dirents are already pinned into the cache, there is no need to scan the
s_children list looking for duplicate Items. The configfs_dirent_exists
check is moved to a location where it is called only during subsystem
initialization.

d_lookup will only need to call configfs_lookup in the case where the
item in question is not pinned to dcache. The only items not pinned to
dcache are attributes. These are placed at the front of the s_children
list, whilst pinned items are inserted at the back. configfs_lookup
stops scanning when it encounters the first pinned entry in s_children.

The assumption of the above optimizations is that there will be few
attributes, but potentially many Items in a given directory.

Signed-off-by: Seamus Connor <[email protected]>
Cc: Joel Becker <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: [email protected]
---

In this revision I've added some commentary describing the changes, and
I have removed a helper function.

fs/configfs/configfs_internal.h | 3 +--
fs/configfs/dir.c | 39 +++++++++++++++++++++++++--------
fs/configfs/inode.c | 24 --------------------
3 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
index c0395363eab9..b91036fd71b1 100644
--- a/fs/configfs/configfs_internal.h
+++ b/fs/configfs/configfs_internal.h
@@ -55,6 +55,7 @@ struct configfs_dirent {
#define CONFIGFS_USET_IN_MKDIR 0x0200
#define CONFIGFS_USET_CREATING 0x0400
#define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
+#define CONFIGFS_PINNED (CONFIGFS_ROOT | CONFIGFS_DIR | CONFIGFS_ITEM_LINK)

extern struct mutex configfs_symlink_mutex;
extern spinlock_t configfs_dirent_lock;
@@ -73,8 +74,6 @@ extern int configfs_make_dirent(struct configfs_dirent *, struct dentry *,
void *, umode_t, int, struct configfs_fragment *);
extern int configfs_dirent_is_ready(struct configfs_dirent *);

-extern void configfs_hash_and_remove(struct dentry * dir, const char * name);
-
extern const unsigned char * configfs_get_name(struct configfs_dirent *sd);
extern void configfs_drop_dentry(struct configfs_dirent *sd, struct dentry *parent);
extern int configfs_setattr(struct user_namespace *mnt_userns,
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index d1f9d2632202..64a0eac83b90 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -207,7 +207,17 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
return ERR_PTR(-ENOENT);
}
sd->s_frag = get_fragment(frag);
- list_add(&sd->s_sibling, &parent_sd->s_children);
+
+ /*
+ * configfs_lookup scans only for unpinned items. s_children is
+ * partitioned so that configfs_lookup can bail out early.
+ * CONFIGFS_PINNED and CONFIGFS_NOT_PINNED are not symmetrical. readdir
+ * cursors still need to be inserted at the front of the list.
+ */
+ if (sd->s_type & CONFIGFS_PINNED)
+ list_add_tail(&sd->s_sibling, &parent_sd->s_children);
+ else
+ list_add(&sd->s_sibling, &parent_sd->s_children);
spin_unlock(&configfs_dirent_lock);

return sd;
@@ -220,10 +230,11 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
*
* called with parent inode's i_mutex held
*/
-static int configfs_dirent_exists(struct configfs_dirent *parent_sd,
- const unsigned char *new)
+static int configfs_dirent_exists(struct dentry *dentry)
{
- struct configfs_dirent * sd;
+ struct configfs_dirent *parent_sd = dentry->d_parent->d_fsdata;
+ const unsigned char *new = dentry->d_name.name;
+ struct configfs_dirent *sd;

list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
if (sd->s_element) {
@@ -289,10 +300,6 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry,

BUG_ON(!item);

- error = configfs_dirent_exists(p->d_fsdata, dentry->d_name.name);
- if (unlikely(error))
- return error;
-
error = configfs_make_dirent(p->d_fsdata, dentry, item, mode,
CONFIGFS_DIR | CONFIGFS_USET_CREATING,
frag);
@@ -449,6 +456,18 @@ static struct dentry * configfs_lookup(struct inode *dir,

spin_lock(&configfs_dirent_lock);
list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+
+ /*
+ * s_children is partitioned, see configfs_new_dirent. The first
+ * pinned item indicates we can stop scanning.
+ */
+ if (sd->s_type & CONFIGFS_PINNED)
+ break;
+
+ /*
+ * Note: CONFIGFS_PINNED and CONFIGFS_NOT_PINNED are asymmetric.
+ * there may be a readdir cursor in this list
+ */
if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
!strcmp(configfs_get_name(sd), dentry->d_name.name)) {
struct configfs_attribute *attr = sd->s_element;
@@ -1878,7 +1897,9 @@ int configfs_register_subsystem(struct configfs_subsystem *subsys)
if (dentry) {
d_add(dentry, NULL);

- err = configfs_attach_group(sd->s_element, &group->cg_item,
+ err = configfs_dirent_exists(dentry);
+ if (!err)
+ err = configfs_attach_group(sd->s_element, &group->cg_item,
dentry, frag);
if (err) {
BUG_ON(d_inode(dentry));
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index b601610e9907..15964e62329d 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -218,27 +218,3 @@ void configfs_drop_dentry(struct configfs_dirent * sd, struct dentry * parent)
}
}

-void configfs_hash_and_remove(struct dentry * dir, const char * name)
-{
- struct configfs_dirent * sd;
- struct configfs_dirent * parent_sd = dir->d_fsdata;
-
- if (d_really_is_negative(dir))
- /* no inode means this hasn't been made visible yet */
- return;
-
- inode_lock(d_inode(dir));
- list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
- if (!sd->s_element)
- continue;
- if (!strcmp(configfs_get_name(sd), name)) {
- spin_lock(&configfs_dirent_lock);
- list_del_init(&sd->s_sibling);
- spin_unlock(&configfs_dirent_lock);
- configfs_drop_dentry(sd, dir);
- configfs_put(sd);
- break;
- }
- }
- inode_unlock(d_inode(dir));
-}
--
2.37.0

2023-10-15 02:00:27

by Joel Becker

[permalink] [raw]
Subject: Re: [PATCH v2] configfs: improve item creation performance

On Fri, Oct 13, 2023 at 02:11:29PM -0700, Seamus Connor wrote:
> As the size of a directory increases item creation slows down.
> Optimizing access to s_children removes this bottleneck.
>
> dirents are already pinned into the cache, there is no need to scan the
> s_children list looking for duplicate Items. The configfs_dirent_exists
> check is moved to a location where it is called only during subsystem
> initialization.
>
> d_lookup will only need to call configfs_lookup in the case where the
> item in question is not pinned to dcache. The only items not pinned to
> dcache are attributes. These are placed at the front of the s_children
> list, whilst pinned items are inserted at the back. configfs_lookup
> stops scanning when it encounters the first pinned entry in s_children.
>
> The assumption of the above optimizations is that there will be few
> attributes, but potentially many Items in a given directory.
>
> Signed-off-by: Seamus Connor <[email protected]>
> Cc: Joel Becker <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: [email protected]
Reviewed-by: Joel Becker <[email protected]>

> ---
>
> In this revision I've added some commentary describing the changes, and
> I have removed a helper function.
>
> fs/configfs/configfs_internal.h | 3 +--
> fs/configfs/dir.c | 39 +++++++++++++++++++++++++--------
> fs/configfs/inode.c | 24 --------------------
> 3 files changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
> index c0395363eab9..b91036fd71b1 100644
> --- a/fs/configfs/configfs_internal.h
> +++ b/fs/configfs/configfs_internal.h
> @@ -55,6 +55,7 @@ struct configfs_dirent {
> #define CONFIGFS_USET_IN_MKDIR 0x0200
> #define CONFIGFS_USET_CREATING 0x0400
> #define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR | CONFIGFS_ITEM_BIN_ATTR)
> +#define CONFIGFS_PINNED (CONFIGFS_ROOT | CONFIGFS_DIR | CONFIGFS_ITEM_LINK)
>
> extern struct mutex configfs_symlink_mutex;
> extern spinlock_t configfs_dirent_lock;
> @@ -73,8 +74,6 @@ extern int configfs_make_dirent(struct configfs_dirent *, struct dentry *,
> void *, umode_t, int, struct configfs_fragment *);
> extern int configfs_dirent_is_ready(struct configfs_dirent *);
>
> -extern void configfs_hash_and_remove(struct dentry * dir, const char * name);
> -
> extern const unsigned char * configfs_get_name(struct configfs_dirent *sd);
> extern void configfs_drop_dentry(struct configfs_dirent *sd, struct dentry *parent);
> extern int configfs_setattr(struct user_namespace *mnt_userns,
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index d1f9d2632202..64a0eac83b90 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -207,7 +207,17 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
> return ERR_PTR(-ENOENT);
> }
> sd->s_frag = get_fragment(frag);
> - list_add(&sd->s_sibling, &parent_sd->s_children);
> +
> + /*
> + * configfs_lookup scans only for unpinned items. s_children is
> + * partitioned so that configfs_lookup can bail out early.
> + * CONFIGFS_PINNED and CONFIGFS_NOT_PINNED are not symmetrical. readdir
> + * cursors still need to be inserted at the front of the list.
> + */
> + if (sd->s_type & CONFIGFS_PINNED)
> + list_add_tail(&sd->s_sibling, &parent_sd->s_children);
> + else
> + list_add(&sd->s_sibling, &parent_sd->s_children);
> spin_unlock(&configfs_dirent_lock);
>
> return sd;
> @@ -220,10 +230,11 @@ static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *paren
> *
> * called with parent inode's i_mutex held
> */
> -static int configfs_dirent_exists(struct configfs_dirent *parent_sd,
> - const unsigned char *new)
> +static int configfs_dirent_exists(struct dentry *dentry)
> {
> - struct configfs_dirent * sd;
> + struct configfs_dirent *parent_sd = dentry->d_parent->d_fsdata;
> + const unsigned char *new = dentry->d_name.name;
> + struct configfs_dirent *sd;
>
> list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
> if (sd->s_element) {
> @@ -289,10 +300,6 @@ static int configfs_create_dir(struct config_item *item, struct dentry *dentry,
>
> BUG_ON(!item);
>
> - error = configfs_dirent_exists(p->d_fsdata, dentry->d_name.name);
> - if (unlikely(error))
> - return error;
> -
> error = configfs_make_dirent(p->d_fsdata, dentry, item, mode,
> CONFIGFS_DIR | CONFIGFS_USET_CREATING,
> frag);
> @@ -449,6 +456,18 @@ static struct dentry * configfs_lookup(struct inode *dir,
>
> spin_lock(&configfs_dirent_lock);
> list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
> +
> + /*
> + * s_children is partitioned, see configfs_new_dirent. The first
> + * pinned item indicates we can stop scanning.
> + */
> + if (sd->s_type & CONFIGFS_PINNED)
> + break;
> +
> + /*
> + * Note: CONFIGFS_PINNED and CONFIGFS_NOT_PINNED are asymmetric.
> + * there may be a readdir cursor in this list
> + */
> if ((sd->s_type & CONFIGFS_NOT_PINNED) &&
> !strcmp(configfs_get_name(sd), dentry->d_name.name)) {
> struct configfs_attribute *attr = sd->s_element;
> @@ -1878,7 +1897,9 @@ int configfs_register_subsystem(struct configfs_subsystem *subsys)
> if (dentry) {
> d_add(dentry, NULL);
>
> - err = configfs_attach_group(sd->s_element, &group->cg_item,
> + err = configfs_dirent_exists(dentry);
> + if (!err)
> + err = configfs_attach_group(sd->s_element, &group->cg_item,
> dentry, frag);
> if (err) {
> BUG_ON(d_inode(dentry));
> diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
> index b601610e9907..15964e62329d 100644
> --- a/fs/configfs/inode.c
> +++ b/fs/configfs/inode.c
> @@ -218,27 +218,3 @@ void configfs_drop_dentry(struct configfs_dirent * sd, struct dentry * parent)
> }
> }
>
> -void configfs_hash_and_remove(struct dentry * dir, const char * name)
> -{
> - struct configfs_dirent * sd;
> - struct configfs_dirent * parent_sd = dir->d_fsdata;
> -
> - if (d_really_is_negative(dir))
> - /* no inode means this hasn't been made visible yet */
> - return;
> -
> - inode_lock(d_inode(dir));
> - list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
> - if (!sd->s_element)
> - continue;
> - if (!strcmp(configfs_get_name(sd), name)) {
> - spin_lock(&configfs_dirent_lock);
> - list_del_init(&sd->s_sibling);
> - spin_unlock(&configfs_dirent_lock);
> - configfs_drop_dentry(sd, dir);
> - configfs_put(sd);
> - break;
> - }
> - }
> - inode_unlock(d_inode(dir));
> -}
> --
> 2.37.0
>

--

"Glory is fleeting, but obscurity is forever."
- Napoleon Bonaparte

http://www.jlbec.org/
[email protected]

2023-10-16 06:08:17

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] configfs: improve item creation performance

Thanks Seamus,

I've applied the patch (with two very minor coding style fixups)
to the configfs for-next branch.

2023-10-16 15:57:06

by Seamus Connor

[permalink] [raw]
Subject: Re: [PATCH v2] configfs: improve item creation performance

Thanks to you both!

-Seamus