All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: "Reshetova, Elena" <elena.reshetova@intel.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"trond.myklebust@primarydata.com"
	<trond.myklebust@primarydata.com>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"keescook@chromium.org" <keescook@chromium.org>
Subject: Re: [PATCH 1/4] lockd: convert nlm_host.h_count from atomic_t to refcount_t
Date: Fri, 22 Dec 2017 10:42:50 -0500	[thread overview]
Message-ID: <20171222154250.GB9630@fieldses.org> (raw)
In-Reply-To: <20171222142553.GA9630@fieldses.org>

On Fri, Dec 22, 2017 at 09:25:53AM -0500, J. Bruce Fields wrote:
> On Fri, Dec 22, 2017 at 09:29:15AM +0000, Reshetova, Elena wrote:
> > 
> > On Wed, Nov 29, 2017 at 01:15:43PM +0200, Elena Reshetova wrote:
> > > atomic_t variables are currently used to implement reference
> > > counters with the following properties:
> > >  - counter is initialized to 1 using atomic_set()
> > >  - a resource is freed upon counter reaching zero
> > >  - once counter reaches zero, its further
> > >    increments aren't allowed
> > >  - counter schema uses basic atomic operations
> > >    (set, inc, inc_not_zero, dec_and_test, etc.)
> > 
> > >Whoops, I forgot that this doesn't apply to h_count.
> > 
> > >Well, it's confusing, because h_count is actually used in two different
> > >ways: depending on whether a nlm_host represents a client or server, it
> > >may have the above properties or not.
> > 
> > 
> > So, what happens when it is not having the above properties? Is the object 
> > being reused or? 
> 
> The object isn't destroyed when the counter hits zero--zero is just
> taken as a hint to some garbage collection algorithm that it would be OK
> to destroy it.  So decrementing to or incrementing from zero is OK.

In more detail: the nlm_host objects that are used on the NFS server to
represent NFS clients are put by nlmsvc_release_host, and then may
eventually be freed by nlm_gc_hosts.

The nlm_host objects that are used on the NFS client to represent NFS
servers are put (and freed when h_count goes to zero) by
nlmclnt_release_host.

In both cases reference are taken by nlm_get_host.  It would be possible
to replace nlm_get_host by two different functions if that would help.
Most callers are obviously only client-side or server-side.  The only
exception is next_host_state.  It could be passed a pointer to the "get"
function it should use.

After that we might actually just want to define separate client and
server structs like:

	struct nlm_clnt_host {
		struct nlm_host ch_host;
		refcount_t	ch_count;
		...
	}

	struct nlm_srv_host {
		struct nlm_host sh_host;
		refcount_t	sh_count;
		...
	}

rather than have a single h_count which is used in two confusingly
different ways.  There are also some other nlm_host fields that really
only make sense for client or server.

--b.

  reply	other threads:[~2017-12-22 15:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29 11:15 [PATCH 0/4] lockd refcount conversions Elena Reshetova
2017-11-29 11:15 ` [PATCH 1/4] lockd: convert nlm_host.h_count from atomic_t to refcount_t Elena Reshetova
2017-12-21 20:23   ` J. Bruce Fields
2017-12-22  9:29     ` Reshetova, Elena
2017-12-22 14:25       ` J. Bruce Fields
2017-12-22 15:42         ` J. Bruce Fields [this message]
2017-12-27 12:10           ` Reshetova, Elena
2018-01-23 22:09             ` J. Bruce Fields
2018-01-24  0:47               ` Trond Myklebust
2018-01-24  0:47                 ` Trond Myklebust
2018-01-24 21:09                 ` J. Bruce Fields
2017-11-29 11:15 ` [PATCH 2/4] lockd: convert nsm_handle.sm_count " Elena Reshetova
2017-11-29 11:15 ` [PATCH 3/4] lockd: convert nlm_lockowner.count " Elena Reshetova
2017-11-29 11:15 ` [PATCH 4/4] lockd: convert nlm_rqst.a_count " Elena Reshetova
2017-11-29 22:23 ` [PATCH 0/4] lockd refcount conversions J. Bruce Fields
2017-11-30  7:48   ` Reshetova, Elena

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=20171222154250.GB9630@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=anna.schumaker@netapp.com \
    --cc=elena.reshetova@intel.com \
    --cc=jlayton@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=trond.myklebust@primarydata.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.