[Cluster-devel] [GFS2 PATCH 3/6] GFS2: Refactor function fast_to_acquire

[Bob Peterson <rpeterso@redhat.com>]

Hi,

Looking at this again: Today's fast_to_acquire function checks:
(1) if the rgrp glock is locked, has no holders and is not being
demoted, it considers it fast to acquire.
(2) If the rgrp glock is "preferred," it STILL considers it fast to
acquire.

So with today's code, even if a "preferred" glock was being demoted,
for example, it would still consider it fast to acquire.

So the replacement patch's logic is not that different from what
we have today; it's just that the "preferred rgrps" case is now
checked first.

So is today's logic wrong then too?

Bob

----- Original Message -----
| Hi,
| 
| 
| On 10/01/18 20:42, Bob Peterson wrote:
| > Function fast_to_acquire determines if a rgrp ought to be fast to
| > acquire, but the logic was cryptic. This patch makes the logic more
| > straightforward:
| >
| > 1. If the rgrp is one of our "preferred" rgrps, consider it fast
| >     to acquire. This is how each node tries to confine its allocations
| >     to a set of rgrps unique from the other nodes, to reduce contention.
| > 2. If the rgrp glock is unlocked, consider it slow, because it will
| >     take some time to lock it, either for the DLM or for the glock
| >     state machine.
| > 3. If there are glock holders, consider it slow because we'd have to
| >     wait for another process to dequeue before we get our turn.
| > 4. If the glock is being demoted, consider it slow.
| >
| > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
| This appears to mean that for preferred rgrps we will no longer take
| into account any of the other conditions. I'm not sure that is right,
| since we want to avoid rgrps that are in use by other nodes, or are
| being demoted, even if they are preferred. The location of the DLM lock
| master is not really that critical, the slow thing is the I/O that is
| done under the glocks,
| 
| Steve.
| 
| > ---
| >   fs/gfs2/rgrp.c | 30 +++++++++++++++++++++---------
| >   1 file changed, 21 insertions(+), 9 deletions(-)
| >
| > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
| > index 960fbc2d39d7..fa72e3e6ce4e 100644
| > --- a/fs/gfs2/rgrp.c
| > +++ b/fs/gfs2/rgrp.c
| > @@ -1938,20 +1938,32 @@ static bool gfs2_select_rgrp(struct gfs2_rgrpd
| > **pos, const struct gfs2_rgrpd *b
| >   /**
| >    * fast_to_acquire - determine if a resource group will be fast to
| >    acquire
| >    *
| > - * If this is one of our preferred rgrps, it should be quicker to acquire,
| > - * because we tried to set ourselves up as dlm lock master.
| > + * 1. If this is one of our preferred rgrps, it should be quicker to
| > acquire,
| > + *    because we tried to set ourselves up as dlm lock master.
| > + * 2. If the glock is unlocked, consider it slow because it will take time
| > for
| > + *    the dlm and the glock state machine to transition it to a locked
| > state.
| > + * 3. If there are glock holder records queued to the glock, consider it
| > slow
| > + *    because this process will need to be queued up behind another
| > process:
| > + *    Our request can't be enqueued until the other is dequeued.
| > + * 4. If the rgrp glock is being demoted, consider it slow because the
| > glock
| > + *    will need to be demoted, possibly used on another node, the
| > promoted,
| > + *    all of which will take time.
| > + *
| >    */
| > -static inline int fast_to_acquire(struct gfs2_rgrpd *rgd)
| > +static inline bool fast_to_acquire(struct gfs2_rgrpd *rgd)
| >   {
| >   	struct gfs2_glock *gl = rgd->rd_gl;
| >   
| > -	if (gl->gl_state != LM_ST_UNLOCKED && list_empty(&gl->gl_holders) &&
| > -	    !test_bit(GLF_DEMOTE_IN_PROGRESS, &gl->gl_flags) &&
| > -	    !test_bit(GLF_DEMOTE, &gl->gl_flags))
| > -		return 1;
| >   	if (rgd->rd_flags & GFS2_RDF_PREFERRED)
| > -		return 1;
| > -	return 0;
| > +		return true;
| > +	if (gl->gl_state == LM_ST_UNLOCKED)
| > +		return false;
| > +	if (!list_empty(&gl->gl_holders))
| > +		return false;
| > +	if (test_bit(GLF_DEMOTE_IN_PROGRESS, &gl->gl_flags) ||
| > +	    test_bit(GLF_DEMOTE, &gl->gl_flags))
| > +		return false;
| > +	return true;
| >   }
| >   
| >   /**
| 
|