Mailing List Archive

On Wed, 31 Oct 2007 11:37:36 +0100,
Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:

> > mask_out() would also imply that the common use case is to have all
> > attributes in the group created and that you need to take action to
> > have an attribute not created.
>
> Here you have a point. But James has a point too when he says:
> | We basically want to show capability by which file is present.

Currently, if you register an attribute group, all of the attributes
will show up. I find it more intuitive to have to block some attributes
explicitely if I want to have more control, but the original logic is
fine with me as well, if most people prefer that :)

> Anyway, /if/ the reverse logic is preferred, it shouldn't be called
> "mask_out()" but rather "is_masked()" or "is_hidden()" or the like.

Sure. is_masked() would be fine.

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

On Tue, 2007-10-30 at 20:55 -0700, Greg KH wrote:
> OSame problem here, if grp->is_visible is not set, sysfs_add_file() would
> never be called, right?
>
> Other than the logic problem (I think), I have no issue with this idea
> at all. Care to redo this so it works?

It's a fair cop govenor ... new patch attached.

James

---
Index: BUILD-2.6/fs/sysfs/group.c
===================================================================
--- BUILD-2.6.orig/fs/sysfs/group.c 2007-10-28 17:27:04.000000000 -0500
+++ BUILD-2.6/fs/sysfs/group.c 2007-10-31 09:29:14.000000000 -0500
@@ -16,25 +16,31 @@
#include "sysfs.h"


-static void remove_files(struct sysfs_dirent *dir_sd,
+static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
+ int i;

- for (attr = grp->attrs; *attr; attr++)
- sysfs_hash_and_remove(dir_sd, (*attr)->name);
+ for (i = 0, attr = grp->attrs; *attr; i++, attr++)
+ if (!grp->is_visible ||
+ grp->is_visible(kobj, *attr, i))
+ sysfs_hash_and_remove(dir_sd, (*attr)->name);
}

-static int create_files(struct sysfs_dirent *dir_sd,
+static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int error = 0;
+ int error = 0, i;

- for (attr = grp->attrs; *attr && !error; attr++)
- error = sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
+ for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++)
+ if (!grp->is_visible ||
+ grp->is_visible(kobj, *attr, i))
+ error |=
+ sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
if (error)
- remove_files(dir_sd, grp);
+ remove_files(dir_sd, kobj, grp);
return error;
}

@@ -54,7 +60,7 @@ int sysfs_create_group(struct kobject *
} else
sd = kobj->sd;
sysfs_get(sd);
- error = create_files(sd, grp);
+ error = create_files(sd, kobj, grp);
if (error) {
if (grp->name)
sysfs_remove_subdir(sd);
@@ -75,7 +81,7 @@ void sysfs_remove_group(struct kobject *
} else
sd = sysfs_get(dir_sd);

- remove_files(sd, grp);
+ remove_files(sd, kobj, grp);
if (grp->name)
sysfs_remove_subdir(sd);

Index: BUILD-2.6/include/linux/sysfs.h
===================================================================
--- BUILD-2.6.orig/include/linux/sysfs.h 2007-10-28 17:20:06.000000000 -0500
+++ BUILD-2.6/include/linux/sysfs.h 2007-10-30 13:24:06.000000000 -0500
@@ -32,6 +32,8 @@ struct attribute {

struct attribute_group {
const char *name;
+ int (*is_visible)(struct kobject *,
+ struct attribute *, int);
struct attribute **attrs;
};

Index: BUILD-2.6/kernel/params.c
===================================================================
--- BUILD-2.6.orig/kernel/params.c 2007-10-29 01:18:43.000000000 -0500
+++ BUILD-2.6/kernel/params.c 2007-10-29 01:18:49.000000000 -0500
@@ -472,7 +472,7 @@ param_sysfs_setup(struct module_kobject
sizeof(mp->grp.attrs[0]));
size[1] = (valid_attrs + 1) * sizeof(mp->grp.attrs[0]);

- mp = kmalloc(size[0] + size[1], GFP_KERNEL);
+ mp = kzalloc(size[0] + size[1], GFP_KERNEL);
if (!mp)
return ERR_PTR(-ENOMEM);



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

On Wed, Oct 31, 2007 at 09:38:04AM -0500, James Bottomley wrote:
> On Tue, 2007-10-30 at 20:55 -0700, Greg KH wrote:
> > OSame problem here, if grp->is_visible is not set, sysfs_add_file() would
> > never be called, right?
> >
> > Other than the logic problem (I think), I have no issue with this idea
> > at all. Care to redo this so it works?
>
> It's a fair cop govenor ... new patch attached.

Much better, thanks :)

I have no problem with this, if you want to take this in your tree,
please do and feel free to add:
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

to it, or I can take it in mine.

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/

On Wed, 2007-10-31 at 10:29 -0700, Greg KH wrote:
> On Wed, Oct 31, 2007 at 09:38:04AM -0500, James Bottomley wrote:
> > On Tue, 2007-10-30 at 20:55 -0700, Greg KH wrote:
> > > OSame problem here, if grp->is_visible is not set, sysfs_add_file() would
> > > never be called, right?
> > >
> > > Other than the logic problem (I think), I have no issue with this idea
> > > at all. Care to redo this so it works?
> >
> > It's a fair cop govenor ... new patch attached.
>
> Much better, thanks :)
>
> I have no problem with this, if you want to take this in your tree,
> please do and feel free to add:
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> to it, or I can take it in mine.

Actually, I think I have a use for it and the follow on patch (coming
soon) to do attribute bitmaps in the SCSI tree, so I'd like to add your
ack and take it through this tree.

Thanks,

James


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

On Sun, Nov 04, 2007 at 08:12:02AM -0600, James Bottomley wrote:
> On Wed, 2007-10-31 at 10:29 -0700, Greg KH wrote:
> > On Wed, Oct 31, 2007 at 09:38:04AM -0500, James Bottomley wrote:
> > > On Tue, 2007-10-30 at 20:55 -0700, Greg KH wrote:
> > > > OSame problem here, if grp->is_visible is not set, sysfs_add_file() would
> > > > never be called, right?
> > > >
> > > > Other than the logic problem (I think), I have no issue with this idea
> > > > at all. Care to redo this so it works?
> > >
> > > It's a fair cop govenor ... new patch attached.
> >
> > Much better, thanks :)
> >
> > I have no problem with this, if you want to take this in your tree,
> > please do and feel free to add:
> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> >
> > to it, or I can take it in mine.
>
> Actually, I think I have a use for it and the follow on patch (coming
> soon) to do attribute bitmaps in the SCSI tree, so I'd like to add your
> ack and take it through this tree.

That's fine. Can you CC: me on the patch that uses this code so I can
see how well it works out?

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/