[PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent - Kernel

This is a discussion on [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent - Kernel ; 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 Date: Wed Jul 16 08:54:36 2008 +1000 Support sysfs_notify from atomic context with new sysfs_notify_dirent sysfs_notify ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent

  1. [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent


    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
    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

    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
    #include
    #include
    +#include

    #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)
    {
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent

    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

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent


    Hi Tejun,

    On Wednesday July 16, teheo@suse.de 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


    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
    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
    Acked-by: Tejun Heo
    ---
    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

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH/RFC] sysfs: Support sysfs_notify from atomic context with new sysfs_notify_dirent

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread