From mboxrd@z Thu Jan 1 00:00:00 1970 From: Louis Rilling Subject: Re: [Ocfs2-devel] [RFC] configfs: Pin configfs subsystems separately from new config_items. Date: Mon, 23 Jun 2008 17:44:57 +0200 Message-ID: <20080623154457.GW30804@localhost> References: <1213742460-26331-1-git-send-email-joel.becker@oracle.com> <20080618123134.GC30804@localhost> <20080618161215.GA16780@ca-server1.us.oracle.com> <20080618165101.GI30804@localhost> <20080618200713.GE16780@ca-server1.us.oracle.com> <20080619111357.GM30804@localhost> <20080619220739.GC10888@mail.oracle.com> <20080620124644.GQ30804@localhost> <20080620223614.GD21416@mail.oracle.com> Reply-To: Louis.Rilling@kerlabs.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=_bohort-8938-1214235804-0001-2" Cc: linux-fsdevel@vger.kernel.org, cluster-devel@redhat.com, ocfs2-devel@oss.oracle.com To: Joel Becker Return-path: Received: from bohort.kerlabs.com ([62.160.40.57]:37928 "EHLO bohort.kerlabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754373AbYFWPo7 (ORCPT ); Mon, 23 Jun 2008 11:44:59 -0400 Content-Disposition: inline In-Reply-To: <20080620223614.GD21416@mail.oracle.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: This is a MIME-formatted message. If you see this text it means that your E-mail software does not support MIME-formatted messages. --=_bohort-8938-1214235804-0001-2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 20, 2008 at 03:36:14PM -0700, Joel Becker wrote: > On Fri, Jun 20, 2008 at 02:46:44PM +0200, Louis Rilling wrote: > > But keeping this last config_item_put() prevents the simplification tha= t I'm > > defending. And I think that the simplification is worth moving this > > config_item_put() before client_drop_item(). >=20 > I guess I'm not seeing what's simpler. Four lines of > try_module_get() aren't very complex, really. Conversely, having some > functions that *don't* do config_item_get_item() is weird. Not invalid, > just harder to read. config_item_get_item() followed by config_item_put(), with a comment on why= this is correct, was my suggestion, not avoiding to call config_item_get_item(). > Let's put it on the shelf. What I have with this patch is > really no different in effective behavior. I'm completely ok with your patch. I just thought that this was the occasio= n to cleanup module pinning stuff. Would you consider the last following argument? Consider a very simple subsystem, implemented by a single module, that does absolutely no config_item_get(). Currently, this module must provide a rele= ase() operation for all user-created config_items, because the last reference is dropped by configfs, right after dropping the item. With my simplification,= the module effectively drops the last reference in ->drop_item(), and could rel= y on that to make the code simpler like below: make_item() new_item =3D kmalloc(); config_item_init_type_long_name(); return new_item; drop_item(item) config_item_put(item); kfree(item); No need for a release() operation in that simple case, and one may find such code more readable thanks to its symmetry. Looking at some in-tree code, netconsole would be a candidate for such code simplification. Indeed, netconsole does not look like needing config_item_g= et(), because the target_list_lock already provides the necessary protection. That said, thanks for the discussion, and for not forgetting the idea :) Louis --=20 Dr Louis Rilling Kerlabs Skype: louis.rilling Batiment Germanium Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes http://www.kerlabs.com/ 35700 Rennes --=_bohort-8938-1214235804-0001-2 Content-Type: application/pgp-signature; name="signature.asc" Content-Transfer-Encoding: 7bit Content-Description: Digital signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFIX8T5VKcRuvQ9Q1QRAnh1AKDU6PVfI81sNFkYUuj4QVYHomgtXQCeIAKA DWs5KMBlPx6zqNl6JcQrasY= =cDUB -----END PGP SIGNATURE----- --=_bohort-8938-1214235804-0001-2--