LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Greg KH <gregkh@suse.de>
Cc: Mark Fasheh <mark.fasheh@oracle.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com
Subject: Re: [git pull] Fix recent Ocfs2 breakage
Date: Tue, 29 Jan 2008 14:18:59 -0800	[thread overview]
Message-ID: <20080129221858.GA27902@mail.oracle.com> (raw)
In-Reply-To: <20080129185448.GA358@suse.de>

On Tue, Jan 29, 2008 at 10:54:48AM -0800, Greg KH wrote:
> > 	ocfs2 kobject usage, or other folks'?  If there's anything in
> > the ocfs2 usage that you are unsure of, feel free to ask!
> 
> Ok, great.  Here's a few questions:
> 
>   - ocfs2/cluster/sys.c creates a single kset, o2cb, that is in
>     /sys/o2cb (and I had moved it to /sys/fs/o2cb/)  In that directory
>     it seems there is only 1 file, O2NM_API_VERSION.  Is that really all
>     that goes into that directory?  If so, this can be cleaned up even
>     further and only a single kobject is needed, a kset is overkill.

	The kset exists so we can put the logmask directory underneath
it, as you notice in the next item.

>   - that kset is then passed to the mlog_sys_init() file to chain
>     another subdirectory off of this.  Here's where the meat of the
>     sysfs files are.  You use a custom kset and show/store operations to
>     describe some bit fields?  You never use the kobject here, but only
>     go off of the name of the attribute file, which is fine.  But again,
>     using a ktype/kset is way overkill for this.  It can be all
>     simplified to only have 1 kobject for the directory, and then the
>     individual attributes can be a kobj_attribute.

	Well, what's a kobj_attribute do differently?  I mean, logmask
is one object, distinct from the o2cb toplevel, and it has multiple
attributes.  That's pretty standard sysfs, no?
	Oh, I see, kobj_attribute wraps struct attribute the way we do
with "mlog_attribute".  Pretty much everyone did it themselves back in
the day, with a set of hand-built "turn struct attribute into struct
foo_attribute" version of show() and store().  But then you have ot wrap
that in our own attribute anyway...well, not for the mlog one, I don't
think, as it keys off the name.  So perhaps we could remove "struct
mlog_attribute" and replace it with "struct kobj_attribute" for a
zero-sum change.  Not pressing, but certainly not a problem.
	But I'm still not understanding what you mean by "1 kobject for
the directory".  We have one kobject for the "logmask" directory, and
then attributes for each log type.  I don't see how that would change.
Are you suggesting that we move the log attributes up one level,
eliminating the "logmask" subdir?  That is, "/sys/fs/o2cb/logmask/DLM"
becomes "/sys/fs/o2cb/DLM"?  That won't work, because then "ls
/sys/fs/o2cb/logmask" is no longer a valid way to discover all the log
masks - "interface_revision" isn't a log mask :-)
	Or do you mean something else?

Joel

-- 

"To announce that there must be no criticism of them president, or
 that we are to stand by the president, right or wrong, is not only
 unpatriotic and servile, but is morally treasonable to the American
 public."
	- Theodore Roosevelt

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

  reply	other threads:[~2008-01-29 22:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-29  3:33 Mark Fasheh
2008-01-29  5:08 ` Greg KH
2008-01-29  5:58   ` Mark Fasheh
2008-01-29 17:50     ` Greg KH
2008-01-29  7:44   ` Joel Becker
2008-01-29 18:54     ` Greg KH
2008-01-29 22:18       ` Joel Becker [this message]
2008-01-29 22:22         ` Joel Becker

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=20080129221858.GA27902@mail.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.fasheh@oracle.com \
    --cc=ocfs2-devel@oss.oracle.com \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [git pull] Fix recent Ocfs2 breakage' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).