Re: [Cluster-devel] [PATCH 02/13] GFS2: Make do_xmote determine its own gh parameter

[Steven Whitehouse]

Hi,


On 19/11/18 21:06, Bob Peterson wrote:

Hi Steve,

- Original Message -


On 19/11/18 13:29, Bob Peterson wrote:

This is another baby step toward a better glock state machine.
Before this patch, do_xmote was called with a gh parameter, but
only for promotes, not demotes. This patch allows do_xmote to
determine the gh autonomously.

Signed-off-by: Bob Peterson 

(snip)


Since gh is apparently only used to get the lock flags, it would make
more sense just to pass the lock flags rather than add in an additional
find_first_waiter() call,

Steve.

Perhaps I didn't put enough info into the comments for this patch.

I need to get rid of the gh parameter in order to make the glock
state machine fully autonomous. In other words, function do_xmote will
become a state in the (stand alone) state machine, which itself does not
require a gh parameter and may be called from several places under
several conditions. The state of the glock will determine that it needs
to call do_xmote, but do_xmote needs to figure it out on its own.
A function can't become a state in this sense. The state in this case is 
the content of struct gfs2_glock, and the

functions define how you get from one state to another,



Before this patch, the caller does indeed know the gh pointer, but in
the future, it will replaced by a generic call to the state machine
which will not know it.

Regards,

Bob Peterson


That is not relevant to the point I was making though. The point is that 
if the flags are passed to do_xmote rather than the gh, then that 
resolves the issue of needing to pass the gh and reduces the amount of 
code, since you can pass 0 flags instead of NULL gh,


Steve.

Re: [Cluster-devel] [PATCH 02/13] GFS2: Make do_xmote determine its own gh parameter

[Bob Peterson]

Hi Steve,

- Original Message -
> 
> 
> On 19/11/18 13:29, Bob Peterson wrote:
> > This is another baby step toward a better glock state machine.
> > Before this patch, do_xmote was called with a gh parameter, but
> > only for promotes, not demotes. This patch allows do_xmote to
> > determine the gh autonomously.
> >
> > Signed-off-by: Bob Peterson 

(snip)

> Since gh is apparently only used to get the lock flags, it would make
> more sense just to pass the lock flags rather than add in an additional
> find_first_waiter() call,
> 
> Steve.

Perhaps I didn't put enough info into the comments for this patch.

I need to get rid of the gh parameter in order to make the glock
state machine fully autonomous. In other words, function do_xmote will
become a state in the (stand alone) state machine, which itself does not
require a gh parameter and may be called from several places under
several conditions. The state of the glock will determine that it needs
to call do_xmote, but do_xmote needs to figure it out on its own.

Before this patch, the caller does indeed know the gh pointer, but in
the future, it will replaced by a generic call to the state machine
which will not know it.

Regards,

Bob Peterson

Re: [Cluster-devel] [PATCH 02/13] GFS2: Make do_xmote determine its own gh parameter

[Steven Whitehouse]

On 19/11/18 13:29, Bob Peterson wrote:

This is another baby step toward a better glock state machine.
Before this patch, do_xmote was called with a gh parameter, but
only for promotes, not demotes. This patch allows do_xmote to
determine the gh autonomously.

Signed-off-by: Bob Peterson 
---
  fs/gfs2/glock.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 692784faa464..5f2156f15f05 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -60,7 +60,7 @@ struct gfs2_glock_iter {
  
  typedef void (*glock_examiner) (struct gfs2_glock * gl);
  
-static void do_xmote(struct gfs2_glock *gl, struct gfs2_holder *gh, unsigned int target);

+static void do_xmote(struct gfs2_glock *gl, unsigned int target);
  
  static struct dentry *gfs2_root;

  static struct workqueue_struct *glock_workqueue;
@@ -486,12 +486,12 @@ static void finish_xmote(struct gfs2_glock *gl, unsigned 
int ret)
/* Unlocked due to conversion deadlock, try again */
case LM_ST_UNLOCKED:
  retry:
-   do_xmote(gl, gh, gl->gl_target);
+   do_xmote(gl, gl->gl_target);
break;
/* Conversion fails, unlock and try again */
case LM_ST_SHARED:
case LM_ST_DEFERRED:
-   do_xmote(gl, gh, LM_ST_UNLOCKED);
+   do_xmote(gl, LM_ST_UNLOCKED);
break;
default: /* Everything else */
fs_err(gl->gl_name.ln_sbd, "wanted %u got %u\n",
@@ -528,17 +528,17 @@ static void finish_xmote(struct gfs2_glock *gl, unsigned 
int ret)
  /**
   * do_xmote - Calls the DLM to change the state of a lock
   * @gl: The lock state
- * @gh: The holder (only for promotes)
   * @target: The target lock state
   *
   */
  
-static void do_xmote(struct gfs2_glock *gl, struct gfs2_holder *gh, unsigned int target)

+static void do_xmote(struct gfs2_glock *gl, unsigned int target)
  __releases(>gl_lockref.lock)
  __acquires(>gl_lockref.lock)
  {
const struct gfs2_glock_operations *glops = gl->gl_ops;
struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
+   struct gfs2_holder *gh = find_first_waiter(gl);
unsigned int lck_flags = (unsigned int)(gh ? gh->gh_flags : 0);
int ret;
  
@@ -659,7 +659,7 @@ __acquires(>gl_lockref.lock)

if (!(gh->gh_flags & (LM_FLAG_TRY | LM_FLAG_TRY_1CB)))
do_error(gl, 0); /* Fail queued try locks */
}
-   do_xmote(gl, gh, gl->gl_target);
+   do_xmote(gl, gl->gl_target);
return;
  }
  
Since gh is apparently only used to get the lock flags, it would make 
more sense just to pass the lock flags rather than add in an additional 
find_first_waiter() call,


Steve.