Hi Greg,
I wonder if you would consider this patch for sysfs. It makes
sysfs_notify functionality more useful to me.
Thanks,
NeilBrown
Author: NeilBrown <[email protected]>
Date: Wed Jul 16 08:54:36 2008 +1000
Support sysfs_notify from atomic context with new sysfs_notify_dirent
sysfs_notify currently takes sysfs_mutex.
This means that it cannot be called in atomic context.
sysfs_mutex is sometimes held over a malloc (sysfs_rename_dir)
so it can block on low memory.
In md I want to be able to notify on a sysfs attribute from
atomic context, and I don't want to block on low memory because I
could be in the writeout path for freeing memory.
So:
- export the "sysfs_dirent" structure along with sysfs_get, sysfs_put
and sysfs_get_dirent so I can get the sysfs_dirent that I want to
notify on and hold it in an md structure.
- split sysfs_notify_dirent out of sysfs_notify so the sysfs_dirent
can be notified on with no blocking (just a spinlock).
Signed-off-by: NeilBrown <[email protected]>
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 8c0e4b9..0ea8458 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -606,6 +606,7 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
return sd;
}
+EXPORT_SYMBOL_GPL(sysfs_get_dirent);
static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
const char *name, struct sysfs_dirent **p_sd)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index e7735f6..c6060e8 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -440,6 +440,22 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
return POLLERR|POLLPRI;
}
+void sysfs_notify_dirent(struct sysfs_dirent *sd)
+{
+ struct sysfs_open_dirent *od;
+
+ spin_lock(&sysfs_open_dirent_lock);
+
+ od = sd->s_attr.open;
+ if (od) {
+ atomic_inc(&od->event);
+ wake_up_interruptible(&od->poll);
+ }
+
+ spin_unlock(&sysfs_open_dirent_lock);
+}
+EXPORT_SYMBOL_GPL(sysfs_notify_dirent);
+
void sysfs_notify(struct kobject *k, char *dir, char *attr)
{
struct sysfs_dirent *sd = k->sd;
@@ -450,19 +466,8 @@ void sysfs_notify(struct kobject *k, char *dir, char *attr)
sd = sysfs_find_dirent(sd, dir);
if (sd && attr)
sd = sysfs_find_dirent(sd, attr);
- if (sd) {
- struct sysfs_open_dirent *od;
-
- spin_lock(&sysfs_open_dirent_lock);
-
- od = sd->s_attr.open;
- if (od) {
- atomic_inc(&od->event);
- wake_up_interruptible(&od->poll);
- }
-
- spin_unlock(&sysfs_open_dirent_lock);
- }
+ if (sd)
+ sysfs_notify_dirent(sd);
mutex_unlock(&sysfs_mutex);
}
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 14f0023..ab343e3 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -16,6 +16,7 @@
#include <linux/mount.h>
#include <linux/pagemap.h>
#include <linux/init.h>
+#include <linux/module.h>
#include "sysfs.h"
@@ -115,3 +116,17 @@ out_err:
sysfs_dir_cachep = NULL;
goto out;
}
+
+#undef sysfs_get
+struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
+{
+ return __sysfs_get(sd);
+}
+EXPORT_SYMBOL_GPL(sysfs_get);
+
+#undef sysfs_put
+void sysfs_put(struct sysfs_dirent *sd)
+{
+ __sysfs_put(sd);
+}
+EXPORT_SYMBOL_GPL(sysfs_put);
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index ce4e15f..389cb3d 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -123,7 +123,7 @@ int sysfs_create_subdir(struct kobject *kobj, const char *name,
struct sysfs_dirent **p_sd);
void sysfs_remove_subdir(struct sysfs_dirent *sd);
-static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
+static inline struct sysfs_dirent *__sysfs_get(struct sysfs_dirent *sd)
{
if (sd) {
WARN_ON(!atomic_read(&sd->s_count));
@@ -131,12 +131,14 @@ static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
}
return sd;
}
+#define sysfs_get(sd) __sysfs_get(sd)
-static inline void sysfs_put(struct sysfs_dirent *sd)
+static inline void __sysfs_put(struct sysfs_dirent *sd)
{
if (sd && atomic_dec_and_test(&sd->s_count))
release_sysfs_dirent(sd);
}
+#define sysfs_put(sd) __sysfs_put(sd)
/*
* inode.c
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 7858eac..3ab58db 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -78,6 +78,8 @@ struct sysfs_ops {
ssize_t (*store)(struct kobject *,struct attribute *,const char *, size_t);
};
+struct sysfs_dirent;
+
#ifdef CONFIG_SYSFS
int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *),
@@ -115,6 +117,11 @@ void sysfs_remove_file_from_group(struct kobject *kobj,
const struct attribute *attr, const char *group);
void sysfs_notify(struct kobject *kobj, char *dir, char *attr);
+void sysfs_notify_dirent(struct sysfs_dirent *sd);
+struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+ const unsigned char *name);
+struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd);
+void sysfs_put(struct sysfs_dirent *sd);
extern int __must_check sysfs_init(void);
@@ -215,6 +222,22 @@ static inline void sysfs_remove_file_from_group(struct kobject *kobj,
static inline void sysfs_notify(struct kobject *kobj, char *dir, char *attr)
{
}
+static inline void sysfs_notify_dirent(struct sysfs_dirent *sd)
+{
+}
+static inline
+struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+ const unsigned char *name)
+{
+ return NULL;
+}
+static inline struct sysfs_dirent sysfs_get(struct sysfs_dirent *sd)
+{
+ return NULL;
+}
+static inline void sysfs_put(struct sysfs_dirent *sd)
+{
+}
static inline int __must_check sysfs_init(void)
{
Hello,
Neil Brown wrote:
> +#undef sysfs_get
> +struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
> +{
> + return __sysfs_get(sd);
> +}
> +EXPORT_SYMBOL_GPL(sysfs_get);
> +
> +#undef sysfs_put
> +void sysfs_put(struct sysfs_dirent *sd)
> +{
> + __sysfs_put(sd);
> +}
> +EXPORT_SYMBOL_GPL(sysfs_put);
I don't think this inline technique is really necessary. Please just
drop the inline versions. Other than that,
Acked-by: Tejun Heo <[email protected]>
BTW, things like this strongly suggest that we really should switch to
sd-based interface instead of combination of kobj and optional name string.
Thanks.
--
tejun
Hi Tejun,
On Wednesday July 16, [email protected] wrote:
>
> I don't think this inline technique is really necessary. Please just
> drop the inline versions. Other than that,
Yep, they don't really need to be inline. I've fixed that.
>
> Acked-by: Tejun Heo <[email protected]>
Thanks.
>
> BTW, things like this strongly suggest that we really should switch to
> sd-based interface instead of combination of kobj and optional name string.
Yes. Once this is in I'll change md over to use it and then see if
anything else could benefit from the change.
Greg: Can you take this one now? Thanks.
NeilBrown
>From 39c1ca9fcdfdf7b9f582d0dfb5e80d8ce0d66f7c Mon Sep 17 00:00:00 2001
From: NeilBrown <[email protected]>
Date: Sun, 20 Jul 2008 16:29:05 +1000
Subject: [PATCH] Support sysfs_notify from atomic context with new sysfs_notify_dirent
sysfs_notify currently takes sysfs_mutex.
This means that it cannot be called in atomic context.
sysfs_mutex is sometimes held over a malloc (sysfs_rename_dir)
so it can block on low memory.
In md I want to be able to notify on a sysfs attribute from
atomic context, and I don't want to block on low memory because I
could be in the writeout path for freeing memory.
So:
- export the "sysfs_dirent" structure along with sysfs_get, sysfs_put
and sysfs_get_dirent so I can get the sysfs_dirent that I want to
notify on and hold it in an md structure.
- split sysfs_notify_dirent out of sysfs_notify so the sysfs_dirent
can be notified on with no blocking (just a spinlock).
Signed-off-by: NeilBrown <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 18 ++++++++++++++++++
fs/sysfs/file.c | 31 ++++++++++++++++++-------------
fs/sysfs/sysfs.h | 15 ---------------
include/linux/sysfs.h | 23 +++++++++++++++++++++++
4 files changed, 59 insertions(+), 28 deletions(-)
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 8c0e4b9..09e40a1 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -30,6 +30,23 @@ DEFINE_SPINLOCK(sysfs_assoc_lock);
static DEFINE_SPINLOCK(sysfs_ino_lock);
static DEFINE_IDA(sysfs_ino_ida);
+struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
+{
+ if (sd) {
+ WARN_ON(!atomic_read(&sd->s_count));
+ atomic_inc(&sd->s_count);
+ }
+ return sd;
+}
+EXPORT_SYMBOL_GPL(sysfs_get);
+
+void sysfs_put(struct sysfs_dirent *sd)
+{
+ if (sd && atomic_dec_and_test(&sd->s_count))
+ release_sysfs_dirent(sd);
+}
+EXPORT_SYMBOL_GPL(sysfs_put);
+
/**
* sysfs_link_sibling - link sysfs_dirent into sibling list
* @sd: sysfs_dirent of interest
@@ -606,6 +623,7 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
return sd;
}
+EXPORT_SYMBOL_GPL(sysfs_get_dirent);
static int create_dir(struct kobject *kobj, struct sysfs_dirent *parent_sd,
const char *name, struct sysfs_dirent **p_sd)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index e7735f6..c6060e8 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -440,6 +440,22 @@ static unsigned int sysfs_poll(struct file *filp, poll_table *wait)
return POLLERR|POLLPRI;
}
+void sysfs_notify_dirent(struct sysfs_dirent *sd)
+{
+ struct sysfs_open_dirent *od;
+
+ spin_lock(&sysfs_open_dirent_lock);
+
+ od = sd->s_attr.open;
+ if (od) {
+ atomic_inc(&od->event);
+ wake_up_interruptible(&od->poll);
+ }
+
+ spin_unlock(&sysfs_open_dirent_lock);
+}
+EXPORT_SYMBOL_GPL(sysfs_notify_dirent);
+
void sysfs_notify(struct kobject *k, char *dir, char *attr)
{
struct sysfs_dirent *sd = k->sd;
@@ -450,19 +466,8 @@ void sysfs_notify(struct kobject *k, char *dir, char *attr)
sd = sysfs_find_dirent(sd, dir);
if (sd && attr)
sd = sysfs_find_dirent(sd, attr);
- if (sd) {
- struct sysfs_open_dirent *od;
-
- spin_lock(&sysfs_open_dirent_lock);
-
- od = sd->s_attr.open;
- if (od) {
- atomic_inc(&od->event);
- wake_up_interruptible(&od->poll);
- }
-
- spin_unlock(&sysfs_open_dirent_lock);
- }
+ if (sd)
+ sysfs_notify_dirent(sd);
mutex_unlock(&sysfs_mutex);
}
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index ce4e15f..ef5ecc1 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -123,21 +123,6 @@ int sysfs_create_subdir(struct kobject *kobj, const char *name,
struct sysfs_dirent **p_sd);
void sysfs_remove_subdir(struct sysfs_dirent *sd);
-static inline struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd)
-{
- if (sd) {
- WARN_ON(!atomic_read(&sd->s_count));
- atomic_inc(&sd->s_count);
- }
- return sd;
-}
-
-static inline void sysfs_put(struct sysfs_dirent *sd)
-{
- if (sd && atomic_dec_and_test(&sd->s_count))
- release_sysfs_dirent(sd);
-}
-
/*
* inode.c
*/
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 7858eac..3ab58db 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -78,6 +78,8 @@ struct sysfs_ops {
ssize_t (*store)(struct kobject *,struct attribute *,const char *, size_t);
};
+struct sysfs_dirent;
+
#ifdef CONFIG_SYSFS
int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *),
@@ -115,6 +117,11 @@ void sysfs_remove_file_from_group(struct kobject *kobj,
const struct attribute *attr, const char *group);
void sysfs_notify(struct kobject *kobj, char *dir, char *attr);
+void sysfs_notify_dirent(struct sysfs_dirent *sd);
+struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+ const unsigned char *name);
+struct sysfs_dirent *sysfs_get(struct sysfs_dirent *sd);
+void sysfs_put(struct sysfs_dirent *sd);
extern int __must_check sysfs_init(void);
@@ -215,6 +222,22 @@ static inline void sysfs_remove_file_from_group(struct kobject *kobj,
static inline void sysfs_notify(struct kobject *kobj, char *dir, char *attr)
{
}
+static inline void sysfs_notify_dirent(struct sysfs_dirent *sd)
+{
+}
+static inline
+struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
+ const unsigned char *name)
+{
+ return NULL;
+}
+static inline struct sysfs_dirent sysfs_get(struct sysfs_dirent *sd)
+{
+ return NULL;
+}
+static inline void sysfs_put(struct sysfs_dirent *sd)
+{
+}
static inline int __must_check sysfs_init(void)
{
--
1.5.6.3
On Sun, Jul 20, 2008 at 04:31:42PM +1000, Neil Brown wrote:
>
> Greg: Can you take this one now? Thanks.
Yes, I'll add this to my queue.
thanks,
greg k-h