[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Cluster-devel] Re: [Ocfs2-devel] [RFC] configfs: Pin configfs subsystems separately from new config_items.

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."



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

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]