public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Paul Menage <menage@google.com>
Cc: Chandra Seetharaman <sekharan@us.ibm.com>,
	akpm@osdl.org, ckrm-tech@lists.sourceforge.net,
	linux-kernel@vger.kernel.org
Subject: Re: [ckrm-tech] [PATCH 0/5] Allow more than PAGESIZE data read in configfs
Date: Tue, 10 Oct 2006 14:58:08 -0700	[thread overview]
Message-ID: <20061010215808.GK7911@ca-server1.us.oracle.com> (raw)
In-Reply-To: <6599ad830610101431j33a5dc55h6878d5bc6db91e85@mail.gmail.com>

On Tue, Oct 10, 2006 at 02:31:43PM -0700, Paul Menage wrote:
> >        NAK.  This forces a complex and inappropriate interface on the
> >majority of users, and doesn't honor configfs' simplicity-first design.
> 
> How is the seq_file interface complex and inappropriate? For the
> configfs clients it's basically a drop-in replacement for sprintf(),
> as Chandra's patches show.

	Well, they now have to learn seq_file.  They now get to assume
that "spewing large amounts of junk" is the default rather than "single
attribute", which is correct.  None of it is relevant for the majority
of correct users.
	It exposes the "I'm a file" knowledge down to the client module.
The entire point of configfs is that the "filesystem bits" are
independant of the "client bits".  To the client, it's an item
hierarchy.  To the user, the interface happens to be a filesystem.
	Technically, the seq_printf() as a drop-in replacement seems to
be functional.  I'm worried about lifetiming, but I think it's OK (what
do I mean?  If I open the file, I'd better not be able to remove the
client module until everything is torn down.  If I close the file, it
had better get all torn down before module_put() so that when
->release() returns, te module can safely be removed.  I *think* this
change satisfies these worries, but it's something that absolutely has
to be done right.  Yes, I'm very paranoid about this).
	My bigger worry is that we haven't solved the write side.  How
does one *set* a large attribute?  It had better not be multiple
attributes.  I know that your module doesn't set it, but hey, we don't
codify that requirement.  Perhaps a patch where we say "if you are a
large display attribute, we'll use seq_file and error on write because
it isn't allowed" but that leaves the old buffer-based approach for
normal-sized read-write attributes.

Joel

-- 

Life's Little Instruction Book #252

	"Take good care of those you love."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

  reply	other threads:[~2006-10-10 21:58 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-10 18:20 [PATCH 0/5] Allow more than PAGESIZE data read in configfs Chandra Seetharaman
2006-10-10 18:20 ` [PATCH 1/5] Fix a module count leak Chandra Seetharaman
2006-10-10 22:17   ` Joel Becker
2006-10-10 18:20 ` [PATCH 2/5] Use seq_file for read side of operations Chandra Seetharaman
2006-10-11  9:12   ` Joel Becker
2006-10-10 18:21 ` [PATCH 3/5] Change configfs_example.c to use the new interface Chandra Seetharaman
2006-10-10 18:21 ` [PATCH 4/5] Change Documentation to reflect " Chandra Seetharaman
2006-10-10 18:21 ` [PATCH 5/5] Change the existing code to use " Chandra Seetharaman
2006-10-10 20:35 ` [PATCH 0/5] Allow more than PAGESIZE data read in configfs Joel Becker
2006-10-10 21:31   ` [ckrm-tech] " Paul Menage
2006-10-10 21:58     ` Joel Becker [this message]
2006-10-10 23:13       ` Chandra Seetharaman
2006-10-11  0:15         ` Joel Becker
2006-10-11  0:49       ` Matt Helsley
2006-10-11  1:28         ` Joel Becker
2006-10-11 22:39           ` Greg KH
2006-10-11 23:26             ` Chandra Seetharaman
2006-10-12  4:17               ` Paul Jackson
2006-10-12 23:51               ` Greg KH
2006-10-13  0:16                 ` Paul Jackson
2006-10-13 23:38                   ` Matt Helsley
2006-10-13 23:40                 ` Matt Helsley
2006-10-13 23:47                   ` Paul Menage
2006-10-14  6:17                   ` Greg KH
2006-10-14 23:14                     ` Matt Helsley
2006-10-16 19:10                 ` Chandra Seetharaman
2006-10-16 20:32                   ` Paul Jackson
2006-10-16 22:29                     ` Chandra Seetharaman
2006-10-17  2:59                       ` Paul Jackson
2006-10-12  2:17             ` Matt Helsley
2006-10-12 23:54               ` Greg KH
2006-10-13  3:22                 ` Matt Helsley
     [not found]           ` <20061011220619.GB7911@ca-server1.us.oracle.com>
     [not found]             ` <1160619516.18766.209.camel@localhost.localdomain>
2006-10-12  7:08               ` Joel Becker
2006-10-12 21:44                 ` Paul Jackson
2006-10-12 22:51                   ` Joel Becker
2006-10-13  0:01                     ` Paul Jackson
2006-10-14  4:40                       ` Greg KH
2006-10-13 23:37                 ` Matt Helsley
2006-10-14  0:09                   ` Joel Becker
2006-10-15  1:06                     ` Matt Helsley
2006-10-15 19:07                     ` Paul Jackson
2006-10-16 19:33                     ` Chandra Seetharaman
2006-10-16 23:07                       ` Joel Becker
2006-10-11 20:19   ` Andrew Morton
2006-10-11 21:41     ` Joel Becker
2006-10-11 22:18     ` Joel Becker
2006-10-11 22:48       ` Andrew Morton
2006-10-11 23:27         ` Chandra Seetharaman
2006-10-14  8:01           ` Greg KH
2006-10-14 19:43             ` Andrew Morton
2006-10-14 20:10               ` Joel Becker
2006-10-16 19:24                 ` Chandra Seetharaman
2006-10-16 23:09                   ` Joel Becker
2006-10-18  0:55                     ` Chandra Seetharaman
2006-10-19 18:42                       ` Joel Becker
2006-10-16 19:16             ` Chandra Seetharaman

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=20061010215808.GK7911@ca-server1.us.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=akpm@osdl.org \
    --cc=ckrm-tech@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=menage@google.com \
    --cc=sekharan@us.ibm.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