Re: [git patches] ocfs2 update

Previous thread: Xilinx: hwicap driver comments by Jiri Slaby on Thursday, February 7, 2008 - 1:08 pm. (25 messages)

Next thread: [PATCH 2.5/3] palm_bk3710: port initialization/probing bugfix by Bartlomiej Zolnierkiewicz on Thursday, February 7, 2008 - 1:50 pm. (2 messages)
From: Mark Fasheh
Date: Thursday, February 7, 2008 - 1:09 pm

Here's one last patch for the merge window. It allows the cluster stack to
negotiate a common protocol version with mounting nodes, as opposed to
requiring an exact match as we do today. The patch itself requires a
protocol change, so it'd be nice to see this upstream with all the other
network changes we did for 2.6.25. I held this patch back from my initial
pull request so that we could a bit more testing on it.
	--Mark

Please pull from 'upstream-linus' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/mfasheh/ocfs2.git upstream-linus

to receive the following updates:

 fs/ocfs2/cluster/tcp_internal.h |   11 ++-
 fs/ocfs2/dlm/dlmapi.h           |    7 +-
 fs/ocfs2/dlm/dlmcommon.h        |   24 +++++-
 fs/ocfs2/dlm/dlmdomain.c        |  195 +++++++++++++++++++++++++++++++++-----
 fs/ocfs2/dlm/dlmfs.c            |   15 +++-
 fs/ocfs2/dlm/userdlm.c          |    5 +-
 fs/ocfs2/dlm/userdlm.h          |    3 +-
 fs/ocfs2/dlmglue.c              |   29 ++++++-
 fs/ocfs2/dlmglue.h              |    1 +
 fs/ocfs2/ocfs2.h                |    1 +
 fs/ocfs2/ocfs2_lockingver.h     |   30 ++++++
 fs/ocfs2/super.c                |    1 +
 12 files changed, 288 insertions(+), 34 deletions(-)
 create mode 100644 fs/ocfs2/ocfs2_lockingver.h

Joel Becker (1):
      ocfs2: Negotiate locking protocol versions.

diff --git a/fs/ocfs2/cluster/tcp_internal.h b/fs/ocfs2/cluster/tcp_internal.h
index b2e832a..d25b9af 100644
--- a/fs/ocfs2/cluster/tcp_internal.h
+++ b/fs/ocfs2/cluster/tcp_internal.h
@@ -38,6 +38,15 @@
  * locking semantics of the file system using the protocol.  It should 
  * be somewhere else, I'm sure, but right now it isn't.
  *
+ * With version 11, we separate out the filesystem locking portion.  The
+ * filesystem now has a major.minor version it negotiates.  Version 11
+ * introduces this negotiation to the o2dlm protocol, and as such the
+ * version here in tcp_internal.h should not need to be bumped for
+ * filesystem locking changes.
+ *
+ * New in version ...
From: Andrew Morton
Date: Thursday, February 7, 2008 - 1:47 pm

On Thu, 7 Feb 2008 12:09:44 -0800

It's somewhat obnoxious that what appears to be a straightforward
compare-two-things-and-return-result function will actually modify one of
the things which it is allegedly comparing.

Please integrate checkpatch into your processes - this one had a few little
glitches.

--

From: Mark Fasheh
Date: Thursday, February 7, 2008 - 2:37 pm

Yeah, a better name would probably help with readability. Joel, how about

FWIW - I've run all patches through checkpatch.pl since your last review.
This one went through a couple cycles of checkpatch actually :) There's
three warnings that I get:

ERROR: "foo * bar" should be "foo *bar"
#70: FILE: fs/ocfs2/dlm/dlmapi.h:200:
+struct dlm_ctxt * dlm_register_domain(const char *domain, u32 key,

WARNING: line over 80 characters
#269: FILE: fs/ocfs2/dlm/dlmdomain.c:813:
+
#&dlm->fs_locking_proto,

WARNING: line over 80 characters
#270: FILE: fs/ocfs2/dlm/dlmdomain.c:814:
+
#&query->fs_proto)) {

total: 1 errors, 2 warnings, 569 lines checked


The "foo * bar" one is from existing code which got moved, and I felt that
leaving them unmodified was cleaner from a patch-reading perspective.

The over 80 characters warnings were ignored as the code seemed more
readable as-is.


I guess a lot of this can be subjective though, so I can be super strict if
you really feel it's necessary.


Thanks,
	--Mark

--
Mark Fasheh
Principal Software Developer, Oracle
mark.fasheh@oracle.com
--

From: Andrew Morton
Date: Thursday, February 7, 2008 - 3:17 pm

On Thu, 7 Feb 2008 13:37:15 -0800


I tend to clean those things up as we go, because it's a free patch.

otoh I see that dlm style is presently space-after-asterisk so there's not

yes, I tend to ignore those warnings unless the mess is really gratuitous or

No, you shouldn't view checkpatch as a things-i-must-do.  It is a
things-i-might-have-missed tool.  If you _meant_ things to be that way then
fine, ignore it.
--

From: Joel Becker
Date: Thursday, February 7, 2008 - 3:29 pm

Even better is to move the update outside the function.  What do
we think of the following?

From d667a08d73cd6659219d21cc57f67882f46e42d1 Mon Sep 17 00:00:00 2001
From: Joel Becker <joel.becker@oracle.com>
Date: Thu, 7 Feb 2008 14:25:11 -0800
Subject: [PATCH] ocfs2: Clean up locking protocol negotiation.

The comparison functions for protocol negotiation (introduced in commit
d24fbcda0c4988322949df3d759f1cfb32b32953) were confusing.
Separate out the comparison and value update parts.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ocfs2/dlm/dlmdomain.c |  102 ++++++++++++++++++++++------------------------
 1 files changed, 49 insertions(+), 53 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
index 638d2eb..de802a7 100644
--- a/fs/ocfs2/dlm/dlmdomain.c
+++ b/fs/ocfs2/dlm/dlmdomain.c
@@ -144,8 +144,6 @@ static int dlm_cancel_join_handler(struct o2net_msg *msg, u32 len, void *data,
 				   void **ret_data);
 static int dlm_exit_domain_handler(struct o2net_msg *msg, u32 len, void *data,
 				   void **ret_data);
-static int dlm_protocol_compare(struct dlm_protocol_version *existing,
-				struct dlm_protocol_version *request);
 
 static void dlm_unregister_domain_handlers(struct dlm_ctxt *dlm);
 
@@ -681,36 +679,48 @@ void dlm_unregister_domain(struct dlm_ctxt *dlm)
 }
 EXPORT_SYMBOL_GPL(dlm_unregister_domain);
 
+/*
+ * Compare a requested locking protocol version against the current one.
+ *
+ * If the major numbers are different, they are incompatible.
+ * If the current minor is greater than the request, they are incompatible.
+ * If the current minor is less than or equal to the request, they are
+ * compatible, and the requester should run at the current minor version.
+ */
+static int dlm_protocol_compatible(struct dlm_protocol_version *existing,
+				   struct dlm_protocol_version *request)
+{
+	if (existing->pv_major != request->pv_major)
+		return 0;
+
+	if (existing->pv_minor > request->pv_minor)
+		return ...
Previous thread: Xilinx: hwicap driver comments by Jiri Slaby on Thursday, February 7, 2008 - 1:08 pm. (25 messages)

Next thread: [PATCH 2.5/3] palm_bk3710: port initialization/probing bugfix by Bartlomiej Zolnierkiewicz on Thursday, February 7, 2008 - 1:50 pm. (2 messages)