On Feb 14, 2008, at 4:34 PM, Pete Wyckoff wrote:

Add --enable option to build a threaded pvfs2-client-core, that is,
an executable linked against pvfs2-threaded.so.

Remove the "--threaded" argument from pvfs2-client.  Whatever version
of client-core that was configured is all that is available at runtime.
It will always be installed as pvfs2-client-core.
---
Makefile.in                          |   15 +++++++--------
configure                            |   17 +++++++++++++++--
configure.in                         |   20 ++++++++++++++++++++
src/apps/kernel/linux/module.mk.in   |    9 ++++++---
src/apps/kernel/linux/pvfs2-client.c | 31 + +-----------------------------
5 files changed, 50 insertions(+), 42 deletions(-)


What does everybody think about this?  I had complained a long time
ago about always getting a pvfs2-client-core and
pvfs2-client-core-threaded during a kernapps build.  The latter
brings in the entire libpvfs2-threaded.a and all the objects for
that.

I don't have a feel for who uses the threaded client core.  The only
way to use it now is to start pvfs2-client with the "--threaded"
option.  So I made the --enable option default to off.  Is there a
reason to want both threaded and non-threaded versions installed?

I think the patch looks great. My only concern is that this will make the threaded client daemon never get used. My view is that we should probably have the threaded version as the default. I think in the general case, the threaded version is going to perform better. Its pretty hard to buy a system that doesn't have at least two cores today, and even on a uni-processor system, the performance of the threaded daemon shouldn't be much worse than the non-threaded.

I originally added the threaded client functionality to see if it would help performance, but it turned out to be buggy. Maybe with Phil's recent thread-safety fixes to the system interfaces, we should revisit making the threaded daemon the default.

I'm not sure that the overhead of a single operation will be affected much, honestly. We can't disable threads and locking entirely, since we need the remount thread running. I think the patch I just committed to fix the segfault in HEAD will likely help with individual operation performance more than disabling threads would. Just my 2c though. Its hard to get a feeling for how much the threaded daemon would help without a thorough performance analysis of the two on a number of different systems. Something I would like to do at some point, but its pretty low on the list.

-sam



Please also correct me on the "dnl Enabling this option ..." section
below.  I won't check any of this in unless I get some good feedback.

My next steps would be to make more of the various threading knobs
configurable at compile time without editing Makefile.in.  There's
lots, and they're a bit messy.

                -- Pete



diff --git a/Makefile.in b/Makefile.in
index 33e123b..2a6b766 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -406,8 +406,7 @@ VISMISCSRC :=
KARMASRC :=
# userland helper programs for kernel drivers
KERNAPPSRC :=
-#
-KERNAPPSTHRSRC :=
+KERNAPPTHRSRC :=
# MISCSRC are sources that don't fall into the other categories
MISCSRC :=
# c files generated from state machines
@@ -498,14 +497,14 @@ MISCOBJS := $(patsubst %.c,%.o, $(filter %.c,$ (MISCSRC)))
MISCDEPENDS := $(patsubst %.c,%.d, $(filter %.c,$(MISCSRC)))

# KERNAPPOBJS is a list of kernel driver userland objects
-KERNAPPOBJS := $(patsubst %.c,%.o, $(filter %.c,$(KERNAPPSRC)))
+KERNAPPOBJS := $(patsubst %.c,%.o, $(filter %.c,$(KERNAPPSRC))) \
+ $(patsubst %.c,%-threaded.o, $(filter %.c,$ (KERNAPPTHRSRC)))
# KERNAPPS is a list of kernel driver userland executables
KERNAPPS := $(patsubst %.c,%, $(filter %.c, $(KERNAPPSRC)))
+KERNAPPSTHR := $(patsubst %.c,%, $(filter %.c, $(KERNAPPTHRSRC)))
# KERNAPPDEPENDS is a list of dependency files for kernel driver userland
# objects
-KERNAPPDEPENDS := $(patsubst %.c,%.d, $(filter %.c,$(KERNAPPSRC)))
-
-KERNAPPSTHR := $(patsubst %.c,%-threaded, $(filter %.c, $ (KERNAPPSTHRSRC))) +KERNAPPDEPENDS := $(patsubst %.c,%.d, $(filter %.c,$(KERNAPPSRC) $ (KERNAPPTHRSRC)))

# VISOBJS is a list of visualization program objects
VISOBJS := $(patsubst %.c,%.o, $(filter %.c,$(VISSRC)))
@@ -943,7 +942,7 @@ just_kmod_install: just_kmod
.PHONY: kmod_install
kmod_install: kmod kernapps just_kmod_install
        install -d $(prefix)/sbin
-       install -m 755 $(KERNAPPS) $(prefix)/sbin
+       install -m 755 $(KERNAPPS) $(KERNAPPSTHR) $(prefix)/sbin
endif

ifneq (,$(LINUX24_KERNEL_SRC))
@@ -964,7 +963,7 @@ just_kmod24_install: just_kmod24
.PHONY: kmod24_install
kmod24_install: kmod24 kernapps just_kmod24_install
        install -d $(prefix)/sbin
-       install -m 755 $(KERNAPPS) $(prefix)/sbin
+       install -m 755 $(KERNAPPS) $(KERNAPPSTHR) $(prefix)/sbin
        install -m 755 src/apps/kernel/linux/mount.pvfs2 $(prefix)/sbin
        @echo ""
        @echo "For improved linux-2.4 support,"
diff --git a/configure b/configure
index fb5ca35..e2e6718 100755
--- a/configure
+++ b/configure
@@ -692,6 +692,7 @@ build_static
REDHAT_RELEASE
NPTL_WORKAROUND
MISC_TROVE_FLAGS
+THREADED_KMOD_HELPER
LINUX_KERNEL_SRC
LINUX24_KERNEL_SRC
LINUX24_KERNEL_MINOR_VER
@@ -1331,6 +1332,7 @@ Optional Features:
--disable-aio-threaded-callbacks Disable use of AIO threaded callbacks
  --disable-kernel-aio    Forcibly disable kernel aio
  --enable-kernel-sendfile    Forcibly enable kernel sendfile
+ --enable-threaded-kmod-helper Use threads in the kernel helper application --enable-fast Disable optional debugging, enable optimizations.
  --enable-strict         Turn on strict compiler warnings
  --enable-verbose-build  Enables full output during build process
@@ -11782,6 +11784,16 @@ _ACEOF

fi

+# Check whether --enable-threaded-kmod-helper was given.
+if test "${enable_threaded_kmod_helper+set}" = set; then
+ enableval=$enable_threaded_kmod_helper; if test "x$enableval" = "xyes" ; then
+    THREADED_KMOD_HELPER=yes
+  fi
+
+fi
+
+
+
BUILD_ABSOLUTE_TOP=`pwd`
SRC_RELATIVE_TOP=$srcdir
SRC_ABSOLUTE_TOP=`cd $srcdir ; pwd`
@@ -18482,6 +18494,7 @@ build_static!$build_static$ac_delim
REDHAT_RELEASE!$REDHAT_RELEASE$ac_delim
NPTL_WORKAROUND!$NPTL_WORKAROUND$ac_delim
MISC_TROVE_FLAGS!$MISC_TROVE_FLAGS$ac_delim
+THREADED_KMOD_HELPER!$THREADED_KMOD_HELPER$ac_delim
LINUX_KERNEL_SRC!$LINUX_KERNEL_SRC$ac_delim
LINUX24_KERNEL_SRC!$LINUX24_KERNEL_SRC$ac_delim
LINUX24_KERNEL_MINOR_VER!$LINUX24_KERNEL_MINOR_VER$ac_delim
@@ -18499,7 +18512,6 @@ GNUC!$GNUC$ac_delim
DB_CFLAGS!$DB_CFLAGS$ac_delim
DB_LIB!$DB_LIB$ac_delim
NEEDS_LIBRT!$NEEDS_LIBRT$ac_delim
-TARGET_OS_DARWIN!$TARGET_OS_DARWIN$ac_delim
_ACEOF

if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 97; then
@@ -18541,6 +18553,7 @@ _ACEOF
ac_delim='%!_!# '
for ac_last_try in false false false false false :; do
  cat >conf$$subs.sed <<_ACEOF
+TARGET_OS_DARWIN!$TARGET_OS_DARWIN$ac_delim
TARGET_OS_LINUX!$TARGET_OS_LINUX$ac_delim
BUILD_BMI_TCP!$BUILD_BMI_TCP$ac_delim
BUILD_GM!$BUILD_GM$ac_delim
@@ -18567,7 +18580,7 @@ LIBOBJS!$LIBOBJS$ac_delim
LTLIBOBJS!$LTLIBOBJS$ac_delim
_ACEOF

- if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 24; then + if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 25; then
    break
  elif $ac_last_try; then
{ { echo "$as_me:$LINENO: error: could not make $CONFIG_STATUS" >&5
diff --git a/configure.in b/configure.in
index bf53e89..40c65a6 100644
--- a/configure.in
+++ b/configure.in
@@ -546,6 +546,26 @@ if test -n "$lk_src" ; then
AC_DEFINE(WITH_LINUX_KMOD, 1, [Define to build for linux kernel module userspace helper.])
fi

+dnl
+dnl Enabling this option links pvfs2-client-core against libpvfs2- threaded.so. +dnl The objects in there were compiled using __GEN_POSIX_LOCKING__ and +dnl __PVFS2_JOB_THREADED__. If enabled, the client core will run with two +dnl extra threads: one for listening to the kernel device, and another +dnl to interact with the network. This has the potential to increase the +dnl rate at which concurrent operations are processed, but has the drawback
+dnl of somewhat higher overhead for a single operation.
+dnl
+dnl Note that even without this option, pvfs2-client-core always requires
+dnl pthreads to run its remount thread.
+dnl
+AC_ARG_ENABLE([threaded-kmod-helper],
+[ --enable-threaded-kmod-helper Use threads in the kernel helper application],
+[ if test "x$enableval" = "xyes" ; then
+    THREADED_KMOD_HELPER=yes
+  fi
+])
+AC_SUBST(THREADED_KMOD_HELPER)
+
dnl PAV configuration needs absolute location of source and build.
dnl Linux-2.6 module needs absolute location of source, and uses the
dnl relative location for soft links for out-of-tree builds.
diff --git a/src/apps/kernel/linux/module.mk.in b/src/apps/kernel/ linux/module.mk.in
index 7e71f5f..6e69bb8 100644
--- a/src/apps/kernel/linux/module.mk.in
+++ b/src/apps/kernel/linux/module.mk.in
@@ -1,11 +1,14 @@
DIR := src/apps/kernel/linux

KERNAPPSRC += \
-       $(DIR)/pvfs2-client-core.c \
        $(DIR)/pvfs2-client.c

-KERNAPPSTHRSRC += \
-       $(DIR)/pvfs2-client-core.c
+# if requested, build a threaded client core
+ifeq (,@THREADED_KMOD_HELPER@)
+KERNAPPSRC += $(DIR)/pvfs2-client-core.c
+else
+KERNAPPTHRSRC += $(DIR)/pvfs2-client-core.c
+endif

ifneq (,$(LINUX24_KERNEL_SRC))
KERNAPPSRC += $(DIR)/mount.pvfs2.c
diff --git a/src/apps/kernel/linux/pvfs2-client.c b/src/apps/kernel/ linux/pvfs2-client.c
index dd38e39..09d2bad 100644
--- a/src/apps/kernel/linux/pvfs2-client.c
+++ b/src/apps/kernel/linux/pvfs2-client.c
@@ -28,7 +28,6 @@

#define PVFS2_CLIENT_CORE_SUFFIX  "-core"
#define PVFS2_CLIENT_CORE_NAME "pvfs2-client" PVFS2_CLIENT_CORE_SUFFIX
-#define PVFS2_CLIENT_CORE_THR_SUFFIX "-threaded"

static char s_client_core_path[PATH_MAX];

@@ -59,7 +58,6 @@ typedef struct
    char *logstamp;
    char *dev_buffer_count;
    char *dev_buffer_size;
-    int threaded;
    char *logtype;
} options_t;

@@ -282,15 +280,7 @@ static int monitor_pvfs2_client(options_t *opts)
        {
            sleep(1);

-            if(opts->threaded)
-            {
- arg_list[0] = PVFS2_CLIENT_CORE_NAME PVFS2_CLIENT_CORE_THR_SUFFIX;
-            }
-            else
-            {
-                arg_list[0] = PVFS2_CLIENT_CORE_NAME;
-            }
-
+            arg_list[0] = PVFS2_CLIENT_CORE_NAME;
            arg_index = 1;
            arg_list[arg_index++] = "-a";
            arg_list[arg_index++] = opts->acache_timeout;
@@ -429,7 +419,6 @@ static void print_help(char *progname)
           "PATH\n");
printf("--logstamp=none|usec|datetime override default log message time stamp format\n"); printf("--logtype=file|syslog specify writing logs to file or syslog\n");
-    printf("--threaded                    use threaded client\n");
}

static void parse_args(int argc, char **argv, options_t *opts)
@@ -460,7 +449,6 @@ static void parse_args(int argc, char **argv, options_t *opts)
        {"gossip-mask",1,0,0},
        {"path",1,0,0},
        {"logstamp",1,0,0},
-        {"threaded",0,0,0},
        {0,0,0,0}
    };

@@ -569,11 +557,6 @@ static void parse_args(int argc, char **argv, options_t *opts)
                    opts->gossip_mask = optarg;
                    break;
                }
-                else if (strcmp("threaded", cur_option) == 0)
-                {
-                    opts->threaded = 1;
-                    break;
-                }
                break;
            case 'h':
          do_help:
@@ -644,17 +627,7 @@ static void parse_args(int argc, char **argv, options_t *opts)

    if (!opts->path)
    {
-        if(opts->threaded)
-        {
-            sprintf(s_client_core_path,
- "%s" PVFS2_CLIENT_CORE_SUFFIX PVFS2_CLIENT_CORE_THR_SUFFIX,
-                    argv[0]);
-        }
-        else
-        {
- sprintf(s_client_core_path, "%s" PVFS2_CLIENT_CORE_SUFFIX,
-                    argv[0]);
-        }
+ sprintf(s_client_core_path, "%s" PVFS2_CLIENT_CORE_SUFFIX, argv[0]);
        opts->path = s_client_core_path;
    }

--
1.5.3.8

_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers


_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to