Re: [PATCH -mm] x86 boot : export boot_params via sysfs (forward to Greg)

[Greg KH <greg@kroah.com>]

On Wed, Dec 12, 2007 at 04:11:19PM +0800, Huang, Ying wrote:
> On Tue, 2007-12-11 at 23:35 -0800, Greg KH wrote:
> > > +static struct attribute *boot_params_attrs[] = {
> > > +	&boot_params_version_attr.attr,
> > > +	NULL
> > > +};
> > > +
> > > +static struct attribute_group boot_params_attr_group = {
> > > +	.attrs = boot_params_attrs,
> > > +};
> > > +
> > > +static ssize_t boot_params_data_read(struct kobject *kobj,
> > > +				     struct bin_attribute *bin_attr,
> > > +				     char *buf, loff_t off, size_t count)
> > > +{
> > > +	memcpy(buf, (void *)&boot_params + off, count);
> > > +	return count;
> > > +}
> > 
> > Why is the whole structure being exported to userspace directly?
> > Traditionally we only use the binary files for blobs that are not
> > interpreted by the kernel in any way.  Is that what this structure
> > really is?
> 
> The struct boot_params is not the case. But it is too complex (such as
> the struct edd_info in it). It is very complex to output every fields
> include the first level fields until the primary data type. And Peter
> Anvin think it is OK to output it as a big binary file.

Well, I respectively disagree.  sysfs is NOT for exporting various
binary kernel structures to userspace directly.  Again, the binary files
in sysfs are for chunks of memory that are PASS-THROUGH from hardware to
userspace, with no kernel intervention at all.

If you really need such a thing, use debugfs, as the only rule for
debugfs is that there is no rules :)

So I do not agree with this change at this time, and do not want to see
it applied to the tree.

If you really need this kind of information exported to userspace
through sysfs, then expand it all, in a one-text-value-per-file
format.  I would have no objection to that.

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/