Re: [PATCH 1/4] PCI/sysfs: Add pci_dev_resource_attr_is_visible() helper

From: "Krzysztof Wilczyński" <kw@linux.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH 1/4] PCI/sysfs: Add pci_dev_resource_attr_is_visible() helper
Date: Fri, 10 Sep 2021 19:21:01 +0200	[thread overview]
Message-ID: <20210910172101.GA1314672@rocinante> (raw)
In-Reply-To: <YTtm4e9a/gS5Swga@kroah.com>

Hi Greg,

[...]
> >   pci_dev_config_attr_is_visible(..., struct bin_attribute *a, ...)
> >   {
> >     a->size = PCI_CFG_SPACE_SIZE;    # <-- set size in global attr
> >     ...
> >   }
> > 
> >   static struct bin_attribute *pci_dev_config_attrs[] = {
> >     &bin_attr_config, NULL,
> >   };
> >   static const struct attribute_group pci_dev_config_attr_group = {
> >     .bin_attrs = pci_dev_config_attrs,
> >     .is_bin_visible = pci_dev_config_attr_is_visible,
> >   };
> > 
> >   pci_device_add
> >     device_add
> >       device_add_attrs
> >         device_add_groups
> >           sysfs_create_groups
> >             internal_create_groups
> >               internal_create_group
> >                 create_files
> >                   grp->is_bin_visible()
> >                   sysfs_add_file_mode_ns
> >                     size = battr->size      # <-- copy size from attr
> >                     __kernfs_create_file(..., size, ...)
> >                       kernfs_new_node
> >                         __kernfs_new_node
> > 
> 
> You can create a dynamic attribute and register that.  I think some
> drivers/busses do that today to handle this type of thing.

Some static attributes users don't set size today or simply set it to 0, so
then we report 0 bytes in userspace for each such attribute via the backing
i-node.

Would you be open to the idea of adding a .size() callback so that static
attributes users could set size using more proper channels, or do you think
leaving it being set to 0 is fine?

	Krzysztof