lkml.org 
[lkml]   [2002]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: writing to sysfs appears to hang

    On Wed, 20 Nov 2002, Jens Axboe wrote:

    > On Wed, Nov 20 2002, Paul Larson wrote:
    > > On Tue, 2002-11-19 at 11:02, Jens Axboe wrote:
    > > > This has been in the deadline-rbtree patches for some time (uses writes
    > > > to sysfs, too).
    > > >
    > > > ===== fs/sysfs/inode.c 1.59 vs edited =====
    > > > --- 1.59/fs/sysfs/inode.c Wed Oct 30 21:27:35 2002
    > > > +++ edited/fs/sysfs/inode.c Fri Nov 8 14:33:59 2002
    > > > @@ -243,7 +243,7 @@
    > > > if (kobj && kobj->subsys)
    > > > ops = kobj->subsys->sysfs_ops;
    > > > if (!ops || !ops->store)
    > > > - return 0;
    > > > + return -EINVAL;
    > > >
    > > > page = (char *)__get_free_page(GFP_KERNEL);
    > > > if (!page)
    > >
    > > No effect, the behaviour is still the same for me.
    >
    > strace the program then, what is happening? What I saw was a program
    > seeing write() return 0, assuming it didn't write anything, rewrite the
    > whole thing. Repeat.
    >
    > I bet this wouldn't happen if just sysfs didn't allow write open of a
    > file that doesn't have any writeable bits. Smells like another sysfs
    > bug. Pat?

    Yes, it is. The attached patch (relative to current BK) should fix it. I'm
    planning on sending Linus an update soon, and this is in it.


    -pat


    --- linux-2.5-virgin/fs/sysfs/inode.c Wed Nov 20 12:13:06 2002
    +++ linux-2.5-kobject/fs/sysfs/inode.c Wed Nov 20 12:15:18 2002
    @@ -23,6 +23,8 @@
    * Please see Documentation/filesystems/sysfs.txt for more information.
    */

    +#undef DEBUG
    +
    #include <linux/list.h>
    #include <linux/init.h>
    #include <linux/pagemap.h>
    @@ -173,17 +175,11 @@
    sysfs_read_file(struct file *file, char *buf, size_t count, loff_t *ppos)
    {
    struct attribute * attr = file->f_dentry->d_fsdata;
    - struct sysfs_ops * ops = NULL;
    - struct kobject * kobj;
    + struct sysfs_ops * ops = file->private_data;
    + struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
    unsigned char *page;
    ssize_t retval = 0;

    - kobj = file->f_dentry->d_parent->d_fsdata;
    - if (kobj && kobj->subsys)
    - ops = kobj->subsys->sysfs_ops;
    - if (!ops || !ops->show)
    - return 0;
    -
    if (count > PAGE_SIZE)
    count = PAGE_SIZE;

    @@ -234,16 +230,11 @@
    sysfs_write_file(struct file *file, const char *buf, size_t count, loff_t *ppos)
    {
    struct attribute * attr = file->f_dentry->d_fsdata;
    - struct sysfs_ops * ops = NULL;
    - struct kobject * kobj;
    + struct sysfs_ops * ops = file->private_data;
    + struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
    ssize_t retval = 0;
    char * page;

    - kobj = file->f_dentry->d_parent->d_fsdata;
    - if (kobj && kobj->subsys)
    - ops = kobj->subsys->sysfs_ops;
    - if (!ops || !ops->store)
    - return -EINVAL;

    page = (char *)__get_free_page(GFP_KERNEL);
    if (!page)
    @@ -275,21 +266,72 @@
    return retval;
    }

    -static int sysfs_open_file(struct inode * inode, struct file * filp)
    +static int check_perm(struct inode * inode, struct file * file)
    {
    - struct kobject * kobj;
    + struct kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
    + struct attribute * attr = file->f_dentry->d_fsdata;
    + struct sysfs_ops * ops = NULL;
    int error = 0;

    - kobj = filp->f_dentry->d_parent->d_fsdata;
    - if ((kobj = kobject_get(kobj))) {
    - struct attribute * attr = filp->f_dentry->d_fsdata;
    - if (!attr)
    - error = -EINVAL;
    - } else
    - error = -EINVAL;
    + if (!kobj || !attr)
    + goto Einval;
    +
    + if (kobj->subsys)
    + ops = kobj->subsys->sysfs_ops;
    +
    + /* No sysfs operations, either from having no subsystem,
    + * or the subsystem have no operations.
    + */
    + if (!ops)
    + goto Eaccess;
    +
    + /* File needs write support.
    + * The inode's perms must say it's ok,
    + * and we must have a store method.
    + */
    + if (file->f_mode & FMODE_WRITE) {
    +
    + if (!(inode->i_mode & S_IWUGO))
    + goto Eperm;
    + if (!ops->store)
    + goto Eaccess;
    +
    + }
    +
    + /* File needs read support.
    + * The inode's perms must say it's ok, and we there
    + * must be a show method for it.
    + */
    + if (file->f_mode & FMODE_READ) {
    + if (!(inode->i_mode & S_IRUGO))
    + goto Eperm;
    + if (!ops->show)
    + goto Eaccess;
    + }
    +
    + /* No error? Great, store the ops in file->private_data
    + * for easy access in the read/write functions.
    + */
    + file->private_data = ops;
    + goto Done;
    +
    + Einval:
    + error = -EINVAL;
    + goto Done;
    + Eaccess:
    + error = -EACCES;
    + goto Done;
    + Eperm:
    + error = -EPERM;
    + Done:
    return error;
    }

    +static int sysfs_open_file(struct inode * inode, struct file * filp)
    +{
    + return check_perm(inode,filp);
    +}
    +
    static int sysfs_release(struct inode * inode, struct file * filp)
    {
    struct kobject * kobj = filp->f_dentry->d_parent->d_fsdata;
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:31    [W:0.026 / U:10.116 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site