From: Tejun Heo <htejun@gmail.com>
To: gregkh@suse.de, dmitry.torokhov@gmail.com,
cornelia.huck@de.ibm.com, oneukum@suse.de, rpurdie@rpsys.net,
stern@rowland.harvard.edu, maneesh@in.ibm.com,
linux-kernel@vger.kernel.org, htejun@gmail.com
Cc: Tejun Heo <htejun@gmail.com>
Subject: [PATCH 2/3] sysfs: slim down sysfs_dirent->s_active
Date: Tue, 29 May 2007 01:24:06 +0900 [thread overview]
Message-ID: <11803694463144-git-send-email-htejun@gmail.com> (raw)
In-Reply-To: <118036944617-git-send-email-htejun@gmail.com>
Make sysfs_dirent->s_active an atomic_t instead of rwsem. This
reduces the size of sysfs_dirent from 136 to 104 on 64bit and from 76
to 60 on 32bit with lock debugging turned off. With lock debugging
turned on the reduction is much larger.
s_active starts at zero and each active reference increments s_active.
Putting a reference decrements s_active. Deactivation subtracts
SD_DEACTIVATED_BIAS which is currently INT_MIN and assumed to be small
enough to make s_active negative. If s_active is negative,
sysfs_get() no longer grants new references. Deactivation succeeds
immediately if there is no active user; otherwise, it waits using a
completion for the last put.
Due to the removal of lockdep tricks, this change makes things less
trickier in release_sysfs_dirent(). As all the complexity is
contained in three s_active functions, I think it's more readable this
way.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
fs/sysfs/dir.c | 74 +++++++++++++++++++++++++++++++++++-------------------
fs/sysfs/sysfs.h | 13 +--------
2 files changed, 50 insertions(+), 37 deletions(-)
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index f5f0b93..40596a0 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -10,6 +10,7 @@
#include <linux/kobject.h>
#include <linux/namei.h>
#include <linux/idr.h>
+#include <linux/completion.h>
#include <asm/semaphore.h>
#include "sysfs.h"
@@ -32,11 +33,24 @@ static DEFINE_IDA(sysfs_ino_ida);
*/
struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd)
{
- if (sd) {
- if (unlikely(!down_read_trylock(&sd->s_active)))
- sd = NULL;
+ if (unlikely(!sd))
+ return NULL;
+
+ while (1) {
+ int v, t;
+
+ v = atomic_read(&sd->s_active);
+ if (unlikely(v < 0))
+ return NULL;
+
+ t = atomic_cmpxchg(&sd->s_active, v, v + 1);
+ if (likely(t == v))
+ return sd;
+ if (t < 0)
+ return NULL;
+
+ cpu_relax();
}
- return sd;
}
/**
@@ -48,8 +62,21 @@ struct sysfs_dirent *sysfs_get_active(st
*/
void sysfs_put_active(struct sysfs_dirent *sd)
{
- if (sd)
- up_read(&sd->s_active);
+ struct completion *cmpl;
+ int v;
+
+ if (unlikely(!sd))
+ return;
+
+ v = atomic_dec_return(&sd->s_active);
+ if (likely(v != SD_DEACTIVATED_BIAS))
+ return;
+
+ /* atomic_dec_return() is a mb(), we'll always see the updated
+ * sd->s_sibling.next.
+ */
+ cmpl = (void *)sd->s_sibling.next;
+ complete(cmpl);
}
/**
@@ -95,17 +122,25 @@ void sysfs_put_active_two(struct sysfs_d
* sysfs_deactivate - deactivate sysfs_dirent
* @sd: sysfs_dirent to deactivate
*
- * Deny new active references and drain existing ones. s_active
- * will be unlocked when the sysfs_dirent is released.
+ * Deny new active references and drain existing ones.
*/
void sysfs_deactivate(struct sysfs_dirent *sd)
{
- down_write_nested(&sd->s_active, SYSFS_S_ACTIVE_DEACTIVATE);
+ DECLARE_COMPLETION_ONSTACK(wait);
+ int v;
+
+ BUG_ON(!list_empty(&sd->s_sibling));
+ sd->s_sibling.next = (void *)&wait;
- /* s_active will be unlocked by the thread doing the final put
- * on @sd. Lie to lockdep.
+ /* atomic_add_return() is a mb(), put_active() will always see
+ * the updated sd->s_sibling.next.
*/
- rwsem_release(&sd->s_active.dep_map, 1, _RET_IP_);
+ v = atomic_add_return(SD_DEACTIVATED_BIAS, &sd->s_active);
+
+ if (v != SD_DEACTIVATED_BIAS)
+ wait_for_completion(&wait);
+
+ INIT_LIST_HEAD(&sd->s_sibling);
}
static int sysfs_alloc_ino(ino_t *pino)
@@ -141,19 +176,6 @@ void release_sysfs_dirent(struct sysfs_d
repeat:
parent_sd = sd->s_parent;
- /* If @sd is being released after deletion, s_active is write
- * locked. If @sd is cursor for directory walk or being
- * released prematurely, s_active has no reader or writer.
- *
- * sysfs_deactivate() lies to lockdep that s_active is
- * unlocked immediately. Lie one more time to cover the
- * previous lie.
- */
- if (!down_write_trylock(&sd->s_active))
- rwsem_acquire(&sd->s_active.dep_map,
- SYSFS_S_ACTIVE_DEACTIVATE, 0, _RET_IP_);
- up_write(&sd->s_active);
-
if (sd->s_type & SYSFS_KOBJ_LINK)
sysfs_put(sd->s_elem.symlink.target_sd);
if (sd->s_type & SYSFS_COPY_NAME)
@@ -213,8 +235,8 @@ struct sysfs_dirent *sysfs_new_dirent(co
goto err_out;
atomic_set(&sd->s_count, 1);
+ atomic_set(&sd->s_active, 0);
atomic_set(&sd->s_event, 1);
- init_rwsem(&sd->s_active);
INIT_LIST_HEAD(&sd->s_children);
INIT_LIST_HEAD(&sd->s_sibling);
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index f8779ea..ae006b0 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -21,7 +21,7 @@ struct sysfs_elem_bin_attr {
*/
struct sysfs_dirent {
atomic_t s_count;
- struct rw_semaphore s_active;
+ atomic_t s_active;
struct sysfs_dirent * s_parent;
struct list_head s_sibling;
struct list_head s_children;
@@ -42,16 +42,7 @@ struct sysfs_dirent {
atomic_t s_event;
};
-/*
- * A sysfs file which deletes another file when written to need to
- * write lock the s_active of the victim while its s_active is read
- * locked for the write operation. Tell lockdep that this is okay.
- */
-enum sysfs_s_active_class
-{
- SYSFS_S_ACTIVE_NORMAL, /* file r/w access, etc - default */
- SYSFS_S_ACTIVE_DEACTIVATE, /* file deactivation */
-};
+#define SD_DEACTIVATED_BIAS INT_MIN
extern struct vfsmount * sysfs_mount;
extern struct kmem_cache *sysfs_dir_cachep;
--
1.4.3.4
next prev parent reply other threads:[~2007-05-28 16:24 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-28 16:24 [PATCHSET 2.6.22-rc2-mm1] sysfs: reduce memory footprint of sysfs_dirent Tejun Heo
2007-05-28 16:24 ` Tejun Heo [this message]
2007-05-29 16:13 ` [PATCH 2/3] sysfs: slim down sysfs_dirent->s_active Cornelia Huck
2007-05-28 16:24 ` [PATCH 1/3] sysfs: move s_active functions to fs/sysfs/dir.c Tejun Heo
2007-05-28 16:24 ` [PATCH 3/3] sysfs: use singly-linked list for sysfs_dirent tree Tejun Heo
2007-05-29 16:04 ` [PATCHSET 2.6.22-rc2-mm1] sysfs: reduce memory footprint of sysfs_dirent Dmitry Torokhov
2007-05-29 16:17 ` Cornelia Huck
2007-05-29 17:23 ` Cornelia Huck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11803694463144-git-send-email-htejun@gmail.com \
--to=htejun@gmail.com \
--cc=cornelia.huck@de.ibm.com \
--cc=dmitry.torokhov@gmail.com \
--cc=gregkh@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=maneesh@in.ibm.com \
--cc=oneukum@suse.de \
--cc=rpurdie@rpsys.net \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.