Re: [PATCH v6 2/3] sysfs: Add a attr_is_visible function to attribute_group

[Dan Williams]

Greg KH wrote:
[..]
> > 
> > Hey Greg,
> > 
> > I wanted to follow up on this and see if you are able to provide more
> > details for reproducing or if you are able to look into it?
> 
> Last I tried this, it still crashed and would not boot either on my
> laptop or my workstation.  I don't know how it is working properly for
> you, what systems have you tried it on?
> 
> I'm not going to be able to look at this for many weeks due to
> conference stuff, so if you want to take the series and test it and
> hopefully catch my error, that would be great, I'd love to move forward
> and get this merged someday.

I mentioned to Lukas that I was working on a "sysfs group visibility"
patch and he pointed me to this thread. I will note that I tried to make
the "hide group if all attributes are invisible" approach work, but
reverted to a "new is_group_visible() callback" approach. I did read
through the thread and try to improve the argument in the changelog
accordingly.

I do admit to liking the cleanliness (not touching 'struct
attribute_group') of the "hide if no visible attribute" approch, but see
the criticism of that alternative below, and let me know if it is
convincing. I tested it locally with the following hack to make the
group disappear every other sysfs_update_group() event:

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 3e817a6f94c6..a5c4e8f3e93b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2022,9 +2022,20 @@ static umode_t cxl_region_target_visible(struct kobject *kobj,
        return 0;
 }
 
+static umode_t cxl_region_target_group_visible(struct kobject *kobj)
+{
+       static u32 visible;
+
+       if (visible++ & 1)
+               return 0755;
+       return 0;
+}
+
 static const struct attribute_group cxl_region_target_group = {
+       .name = "target_group",
        .attrs = target_attrs,
        .is_visible = cxl_region_target_visible,
+       .is_group_visible = cxl_region_target_group_visible,
 };
 
 static const struct attribute_group *get_cxl_region_target_group(void)


-- >8 --