public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: "Bryan O'Sullivan" <bos@pathscale.com>
Cc: Roland Dreier <rdreier@cisco.com>,
	rolandd@cisco.com, akpm@osdl.org, davem@davemloft.net,
	linux-kernel@vger.kernel.org, openib-general@openib.org
Subject: Revenge of the sysfs maintainer! (was Re: [PATCH 8 of 20] ipath - sysfs support for core driver)
Date: Thu, 9 Mar 2006 16:35:13 -0800	[thread overview]
Message-ID: <20060310003513.GA17050@suse.de> (raw)
In-Reply-To: <1141947143.10693.40.camel@serpentine.pathscale.com>

On Thu, Mar 09, 2006 at 03:32:23PM -0800, Bryan O'Sullivan wrote:
> On Thu, 2006-03-09 at 15:18 -0800, Roland Dreier wrote:
> >  > +static ssize_t show_atomic_stats(struct device_driver *dev, char *buf)
> >  > +{
> >  > +	memcpy(buf, &ipath_stats, sizeof(ipath_stats));
> >  > +
> >  > +	return sizeof(ipath_stats);
> >  > +}
> > 
> > I think putting a whole binary struct in a sysfs attribute is
> > considered a no-no.
> 
> Grumble.

Grumble?  Oh come on, don't export binary structures through sysfs, it's
in the DOCUMENTATION THAT SYSFS IS FOR TEXT FILES ONLY!!!!

If you don't want to export a text file, then use something else other
than sysfs, it's that simple.

> it's a fairly small struct, much less than a page in length,
> and userspace needs an atomic view of it, instead of reading each of the
> umpteen broken-out files that we also provide for humean-readable access
> to each counter.
> 
> I didn't see any point to implementing the sysfs binary file interface
> in order to do exactly what this 6-line function does.  Still don't, in
> fact :-)

sysfs binary files are for PASS-THROUGH things ONLY!  stuff like this is
NOT for sysfs binary files, so even if you switched to using it, it
would not be allowed.

And if I sound grumpy about this whole thing, I am.  I'm tired of people
trying to abuse sysfs and putting crappy userspace APIs in it.  They
don't realize how messy it causes things to be over the long run.  If
you want to make your own filesystem to export stuff in whatever way you
want, feel free to do so (only takes about 200 lines including comments
to do so.)  But DO NOT ABUSE SYSFS just because you don't happen to
agree with the way it was designed, or feel inconvienced by it.

Ok, here's a new rule to help this from happening again in the future:

  If you want to add a new sysfs file to the kernel, it MUST be
  accompanied with full documentation that explains exactly what that
  file contains and what it is for.  No exceptions will be allowed.

Structure for this documentation will be in the format that I layed out
last week, I'll update it again and post it to lkml for review later
tonight.

thanks,

greg k-h

  reply	other threads:[~2006-03-10  0:35 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <ef8042c934401522ed3f.1141922821@localhost.localdomain>
2006-03-09 23:18 ` [PATCH 8 of 20] ipath - sysfs support for core driver Roland Dreier
2006-03-09 23:32   ` Bryan O'Sullivan
2006-03-10  0:35     ` Greg KH [this message]
2006-03-10  0:46       ` Revenge of the sysfs maintainer! (was Re: [PATCH 8 of 20] ipath - sysfs support for core driver) Bryan O'Sullivan
2006-03-10  1:00         ` Greg KH
2006-03-10  4:58           ` Bryan O'Sullivan
2006-03-10  6:34             ` Greg KH
2006-03-10 15:53               ` Dave Jones
2006-03-10  7:57             ` Arjan van de Ven
2006-03-10 13:51               ` Bryan O'Sullivan
2006-03-10 14:06                 ` Arjan van de Ven
2006-03-10 15:55                   ` Bryan O'Sullivan
2006-03-10 16:24                     ` Arjan van de Ven
2006-03-10 16:36                       ` Bryan O'Sullivan
2006-03-10 16:41                         ` Arjan van de Ven
2006-03-10 16:25                     ` Dave Jones
2006-03-10 16:35                       ` Bryan O'Sullivan
2006-03-10 16:48                       ` Greg KH
2006-03-10 16:49                         ` Bryan O'Sullivan
2006-03-10 15:55                 ` Dave Jones
2006-03-10 13:58             ` [openib-general] " Talpey, Thomas
2006-03-09 23:46   ` [PATCH 8 of 20] ipath - sysfs support for core driver Greg KH
2006-03-09 23:48     ` Roland Dreier
2006-03-09 23:59     ` Bryan O'Sullivan
2006-03-10  1:02       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060310003513.GA17050@suse.de \
    --to=gregkh@suse.de \
    --cc=akpm@osdl.org \
    --cc=bos@pathscale.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=openib-general@openib.org \
    --cc=rdreier@cisco.com \
    --cc=rolandd@cisco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox