linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joel Becker <Joel.Becker@oracle.com>
To: Louis Rilling <Louis.Rilling@kerlabs.com>
Cc: linux-fsdevel@vger.kernel.org, cluster-devel@redhat.com,
	ocfs2-devel@oss.oracle.com
Subject: Re: [Ocfs2-devel] [RFC] configfs: Pin configfs subsystems separately from new config_items.
Date: Thu, 19 Jun 2008 15:07:39 -0700	[thread overview]
Message-ID: <20080619220739.GC10888@mail.oracle.com> (raw)
In-Reply-To: <20080619111357.GM30804@localhost>

On Thu, Jun 19, 2008 at 01:13:57PM +0200, Louis Rilling wrote:
> On Wed, Jun 18, 2008 at 01:07:13PM -0700, Joel Becker wrote:
> > > 4/ make_item()/make_group() pins the module of the new item/group if it differs
> > >    from the current one, and at least until drop_item() (must check in-tree
> > >    subsystems, but I suspect that module dependency tracking already does the
> > >    job).
> > 
> > 	This is a silly rule, though.  Why "pin if it is different" when
> > "always pin" is just as effective and much easier to understand?  You
> > never have to ask "but was the item's module pinned?" when tracking a
> > problem.
> 
> Not so silly, if you consider that this relieves in-tree users from having to
> add try_module_get() in their code. Only special users (like me) who create
> items implemented by other modules, without explicitly depending on symbols of
> these modules or keeping references after ->drop_item(), have to deal with
> such module pinning.

	With my rule ("always pin"), single-module users don't have to
try_module_get() at all.  Just like today.  That's kind of my point.

> 	And I think that we can also get rid of the last config_item_put() (or
> put it before client_drop_item()), because after client_drop_item() rmdir() does
> not need the item anymore, and client_drop_item() is supposed to call
> config_item_put() (in parent's drop_item() or directly). IOW, when entering
> rmdir() configfs already holds the item's ref that was given by parent's
> ->make_item(), and rmdir() drops that ref in client_drop_item(). No need to hold
> the extra one grabbed by configfs_get_config_item().

	We could, but it's a much cleaner read to hold a reference for
the duration of the function.  And since we hold a module reference
anyway, I like simpler and clearer.

> > 	In the end, we are holding a reference to the module while we
> > are accessing it.  It's overkill for the common case (single module was
> > safe before when we pinned item->owner, and it is safe if we only
> > pin subsys->owner), but it provides the best proper safety if we allow
> > multi-module subsystems.
> 
> As I said above, the way it is done currently, pinning the new item's module
> does not provide any safety in multi-module subsystems.

	We provide safety for ourselves.  We can't provide safety for
the subsystem - we don't know how it is put together.  Once again, the
module reference is just configfs saying "I know that I have a reference
and that I'm safe to access this."

Joel

-- 

To spot the expert, pick the one who predicts the job will take the
longest and cost the most.

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

  reply	other threads:[~2008-06-19 22:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-17 22:41 [RFC] configfs: Pin configfs subsystems separately from new config_items Joel Becker
2008-06-18 12:31 ` [Ocfs2-devel] " Louis Rilling
2008-06-18 16:12   ` Joel Becker
2008-06-18 16:51     ` Louis Rilling
2008-06-18 20:07       ` Joel Becker
2008-06-19 11:13         ` [Ocfs2-devel] " Louis Rilling
2008-06-19 22:07           ` Joel Becker [this message]
2008-06-20 12:46             ` Louis Rilling
2008-06-20 22:36               ` Joel Becker
2008-06-23 15:44                 ` Louis Rilling
2008-06-23 19:10                   ` Joel Becker
2008-06-24  5:04                     ` Louis Rilling
2008-06-24 17:03                       ` 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=20080619220739.GC10888@mail.oracle.com \
    --to=joel.becker@oracle.com \
    --cc=Louis.Rilling@kerlabs.com \
    --cc=cluster-devel@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=ocfs2-devel@oss.oracle.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;
as well as URLs for NNTP newsgroup(s).