Navigation Menu

Skip to content

Commit

Permalink
Fix deadlock when handling a P9_TFLUSH (issue 44)
Browse files Browse the repository at this point in the history
This was a regression introduced recently with the threadpool
reorganization.  A lock was not dropped on successful flushing of
a request, resulting in server deadlock.  Added regression test
kern/t35 for this.

Also added a test user/t15 for another tflush problem (issue 45)
  • Loading branch information
garlick committed May 27, 2011
1 parent 6c0cd8f commit f3835a4
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 10 deletions.
2 changes: 2 additions & 0 deletions libnpfs/fcall.c
Expand Up @@ -262,6 +262,7 @@ np_flush(Npreq *req, Npfcall *tc)
np_req_unref(creq);
ret = np_create_rflush();
creq = NULL;
xpthread_mutex_unlock(&tp->lock);
goto done;
}
}
Expand All @@ -279,6 +280,7 @@ np_flush(Npreq *req, Npfcall *tc)
req->flushreq = creq->flushreq;
creq->flushreq = req;
xpthread_mutex_unlock(&creq->lock);
xpthread_mutex_unlock(&tp->lock);
goto done;
}
creq = creq->next;
Expand Down
2 changes: 1 addition & 1 deletion tests/kern/Makefile.am
Expand Up @@ -22,7 +22,7 @@ TESTS_ENVIRONMENT += "PATH_DIODMOUNT=$(top_builddir)/utils/diodmount"
TESTS_ENVIRONMENT += "./runtest"

TESTS = t05 t06 t07 t13 t14 t15 t16 t17 t18 t19 t22 \
t23 t24 t25 t26 t27 t28 t29 t30 t31 t32 t33 t34
t23 t24 t25 t26 t27 t28 t29 t30 t31 t32 t33 t34 t35

#XFAIL_TESTS =

Expand Down
2 changes: 1 addition & 1 deletion tests/kern/Makefile.in
Expand Up @@ -338,7 +338,7 @@ SUBDIRS = fstest $(am__append_1)
TESTS_ENVIRONMENT = env "PATH_DIOD=$(top_builddir)/diod/diod" \
"PATH_DIODMOUNT=$(top_builddir)/utils/diodmount" "./runtest"
TESTS = t05 t06 t07 t13 t14 t15 t16 t17 t18 t19 t22 \
t23 t24 t25 t26 t27 t28 t29 t30 t31 t32 t33 t34
t23 t24 t25 t26 t27 t28 t29 t30 t31 t32 t33 t34 t35


#XFAIL_TESTS =
Expand Down
1 change: 1 addition & 0 deletions tests/kern/README
Expand Up @@ -38,3 +38,4 @@ t31 Run fstest with server --no-userdb option (takes a couple minutes)
t32 Run dbench for one minute.
t33 Run postmark (takes about a minute)
t34 Run an svnadmin create, which uncovered a bug in fsync
t35 Trigger a tflush by aborting a streaming write.
5 changes: 5 additions & 0 deletions tests/kern/runtest
Expand Up @@ -53,6 +53,11 @@ if [ "`basename $TEST`" = "t34" ] && ! which svnadmin >/dev/null 2>&1; then
echo "svnadmin is not installed" >$TEST.out
exit 77
fi
# t35 requires scrub
if [ "`basename $TEST`" = "t35" ] && ! which scrub >/dev/null 2>&1; then
echo "scrub is not installed" >$TEST.out
exit 77
fi

ulimit -c unlimited

Expand Down
14 changes: 14 additions & 0 deletions tests/kern/t35
@@ -0,0 +1,14 @@
#!/bin/bash

# svnadmin create $PATH_MNTDIR/svnroot

echo creating 100mb file
dd if=/dev/zero of=$PATH_MNTDIR/file bs=1024k count=100 status=noxfer
echo scrubbing
scrub -s 100m $PATH_MNTDIR/file >/dev/null &
echo interrupting
sleep 0.5
kill -15 %1
wait %1 >/dev/null 2>&1

exit 0
8 changes: 8 additions & 0 deletions tests/kern/t35.exp
@@ -0,0 +1,8 @@
kconjoin: diodmount exited with rc=0
creating 100mb file
100+0 records in
100+0 records out
scrubbing
interrupting
kconjoin: t35 exited with rc=0
kconjoin: diod exited with rc=0
7 changes: 5 additions & 2 deletions tests/user/Makefile.am
Expand Up @@ -6,14 +6,16 @@ check_PROGRAMS = \
tread \
tstat \
twrite \
tcreate
tcreate \
tflush

TESTS_ENVIRONMENT = env
TESTS_ENVIRONMENT += "PATH_DIOD=$(top_builddir)/diod/diod"
TESTS_ENVIRONMENT += "PATH_DIODCONF=$(top_builddir)/etc/diod.conf"
TESTS_ENVIRONMENT += "./runtest"

TESTS = t01 t02 t03 t04 t05 t06 t07 t08 t09 t10 t11 t12 t13 t14
TESTS = t01 t02 t03 t04 t05 t06 t07 t08 t09 t10 t11 t12 t13 t14 t15
XFAIL_TESTS=t15

$(TESTS): exp.d

Expand Down Expand Up @@ -46,6 +48,7 @@ tread_SOURCES = tread.c $(common_sources)
tstat_SOURCES = tstat.c $(common_sources)
twrite_SOURCES = twrite.c $(common_sources)
tcreate_SOURCES = tcreate.c $(common_sources)
tflush_SOURCES = tflush.c $(common_sources)

clean: clean-am
-rm -rf exp.d
Expand Down
27 changes: 21 additions & 6 deletions tests/user/Makefile.in
Expand Up @@ -36,7 +36,7 @@ host_triplet = @host@
target_triplet = @target@
check_PROGRAMS = conjoin$(EXEEXT) tattach$(EXEEXT) tattachmt$(EXEEXT) \
tmkdir$(EXEEXT) tread$(EXEEXT) tstat$(EXEEXT) twrite$(EXEEXT) \
tcreate$(EXEEXT)
tcreate$(EXEEXT) tflush$(EXEEXT)
subdir = tests/user
DIST_COMMON = README $(srcdir)/Makefile.am $(srcdir)/Makefile.in
ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
Expand Down Expand Up @@ -92,6 +92,15 @@ tcreate_DEPENDENCIES = $(top_builddir)/libdiod/libdiod.a \
$(top_builddir)/liblsd/liblsd.a $(am__DEPENDENCIES_1) \
$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
$(am__DEPENDENCIES_1)
am_tflush_OBJECTS = tflush.$(OBJEXT) $(am__objects_1)
tflush_OBJECTS = $(am_tflush_OBJECTS)
tflush_LDADD = $(LDADD)
tflush_DEPENDENCIES = $(top_builddir)/libdiod/libdiod.a \
$(top_builddir)/libnpclient/libnpclient.a \
$(top_builddir)/libnpfs/libnpfs.a \
$(top_builddir)/liblsd/liblsd.a $(am__DEPENDENCIES_1) \
$(am__DEPENDENCIES_1) $(am__DEPENDENCIES_1) \
$(am__DEPENDENCIES_1)
am_tmkdir_OBJECTS = tmkdir.$(OBJEXT) $(am__objects_1)
tmkdir_OBJECTS = $(am_tmkdir_OBJECTS)
tmkdir_LDADD = $(LDADD)
Expand Down Expand Up @@ -137,11 +146,11 @@ COMPILE = $(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \
CCLD = $(CC)
LINK = $(CCLD) $(AM_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@
SOURCES = $(conjoin_SOURCES) tattach.c $(tattachmt_SOURCES) \
$(tcreate_SOURCES) $(tmkdir_SOURCES) $(tread_SOURCES) \
$(tstat_SOURCES) $(twrite_SOURCES)
$(tcreate_SOURCES) $(tflush_SOURCES) $(tmkdir_SOURCES) \
$(tread_SOURCES) $(tstat_SOURCES) $(twrite_SOURCES)
DIST_SOURCES = $(conjoin_SOURCES) tattach.c $(tattachmt_SOURCES) \
$(tcreate_SOURCES) $(tmkdir_SOURCES) $(tread_SOURCES) \
$(tstat_SOURCES) $(twrite_SOURCES)
$(tcreate_SOURCES) $(tflush_SOURCES) $(tmkdir_SOURCES) \
$(tread_SOURCES) $(tstat_SOURCES) $(twrite_SOURCES)
ETAGS = etags
CTAGS = ctags
am__tty_colors = \
Expand Down Expand Up @@ -278,7 +287,8 @@ top_builddir = @top_builddir@
top_srcdir = @top_srcdir@
TESTS_ENVIRONMENT = env "PATH_DIOD=$(top_builddir)/diod/diod" \
"PATH_DIODCONF=$(top_builddir)/etc/diod.conf" "./runtest"
TESTS = t01 t02 t03 t04 t05 t06 t07 t08 t09 t10 t11 t12 t13 t14
TESTS = t01 t02 t03 t04 t05 t06 t07 t08 t09 t10 t11 t12 t13 t14 t15
XFAIL_TESTS = t15
CLEANFILES = *.out *.diff *.diod
AM_CFLAGS = @GCCWARN@
AM_CPPFLAGS = \
Expand All @@ -301,6 +311,7 @@ tread_SOURCES = tread.c $(common_sources)
tstat_SOURCES = tstat.c $(common_sources)
twrite_SOURCES = twrite.c $(common_sources)
tcreate_SOURCES = tcreate.c $(common_sources)
tflush_SOURCES = tflush.c $(common_sources)
EXTRA_DIST = $(TESTS) $(TESTS:%=%.exp) runtest
all: all-am

Expand Down Expand Up @@ -351,6 +362,9 @@ tattachmt$(EXEEXT): $(tattachmt_OBJECTS) $(tattachmt_DEPENDENCIES)
tcreate$(EXEEXT): $(tcreate_OBJECTS) $(tcreate_DEPENDENCIES)
@rm -f tcreate$(EXEEXT)
$(LINK) $(tcreate_OBJECTS) $(tcreate_LDADD) $(LIBS)
tflush$(EXEEXT): $(tflush_OBJECTS) $(tflush_DEPENDENCIES)
@rm -f tflush$(EXEEXT)
$(LINK) $(tflush_OBJECTS) $(tflush_LDADD) $(LIBS)
tmkdir$(EXEEXT): $(tmkdir_OBJECTS) $(tmkdir_DEPENDENCIES)
@rm -f tmkdir$(EXEEXT)
$(LINK) $(tmkdir_OBJECTS) $(tmkdir_LDADD) $(LIBS)
Expand All @@ -374,6 +388,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tattach.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tattachmt.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tcreate.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tflush.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tmkdir.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tread.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/tstat.Po@am__quote@
Expand Down
2 changes: 2 additions & 0 deletions tests/user/README
Expand Up @@ -20,6 +20,8 @@ t10 Attach to server with N threads, 1 user
t11(*) Attach to server with N threads, M users (N > M)
t12(+) Attach to user A's private server with user B
t13(*) Attach to server with N threads, M users (N < M)
t14 Try to create a file with a bogus gid.
t15 Check that flush works the way it ought to.


(*) requires root (else NOTRUN)
Expand Down
3 changes: 3 additions & 0 deletions tests/user/t15
@@ -0,0 +1,3 @@
#!/bin/bash

./tflush "$@"
5 changes: 5 additions & 0 deletions tests/user/t15.exp
@@ -0,0 +1,5 @@
tflush: sent 100 tversions
tflush: sent 1 tflush
tflush: received 100/101 responses
conjoin: t15 exited with rc=0
conjoin: diod exited with rc=0
152 changes: 152 additions & 0 deletions tests/user/tflush.c
@@ -0,0 +1,152 @@
/* tflush.c - issue 100 requests, flush one, read the responses */

#if HAVE_CONFIG_H
#include "config.h"
#endif
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <pthread.h>
#include <stdarg.h>
#include <errno.h>
#include <stdint.h>
#include <inttypes.h>
#include <assert.h>
#include <signal.h>

#include "9p.h"
#include "npfs.h"
#include "npclient.h"
#include "npcimpl.h"

#include "diod_log.h"
#include "diod_auth.h"

static void
_flush_series (Npcfsys *fs, Npcfid *root);

static void
usage (void)
{
fprintf (stderr, "Usage: tflush aname\n");
exit (1);
}

int
main (int argc, char *argv[])
{
Npcfsys *fs;
Npcfid *afid, *root;
char *aname;
int fd = 0; /* stdin */
uid_t uid = geteuid ();

diod_log_init (argv[0]);

if (argc != 2)
usage ();
aname = argv[1];

if (!(fs = npc_start (fd, 8192+24, 0)))
errn_exit (np_rerror (), "npc_start");
if (!(afid = npc_auth (fs, aname, uid, diod_auth)) && np_rerror () != 0)
errn_exit (np_rerror (), "npc_auth");
if (!(root = npc_attach (fs, afid, aname, uid)))
errn_exit (np_rerror (), "npc_attach");
if (afid && npc_clunk (afid) < 0)
errn_exit (np_rerror (), "npc_clunk afid");

_flush_series (fs, root);

if (npc_clunk (root) < 0)
errn_exit (np_rerror (), "npc_clunk root");
npc_finish (fs);

diod_log_fini ();

exit (0);
}

static void
_alarm_clock (int sig)
{
//msg ("alarm clock!");
}

static void
_flush_series (Npcfsys *fs, Npcfid *root)
{
Npfcall *rc = NULL, *tc = NULL, *ac = NULL;
u16 tag, flushtag;
int n, i;
struct sigaction sa;

assert (fs->trans != NULL);

sa.sa_flags = 0;
sigemptyset(&sa.sa_mask);
sa.sa_handler = _alarm_clock;
if (sigaction (SIGALRM, &sa, NULL) < 0)
err_exit ("sigaction");

/* write 100 tversions */
for (i = 0; i < 100; i++) {
if (!(tc = np_create_tversion (fs->msize, "9P2000.L")))
msg_exit ("out of memory");
flushtag = tag = npc_get_id(fs->tagpool);
np_set_tag(tc, tag);
n = np_trans_write(fs->trans, tc->pkt, tc->size);
if (n < 0)
errn_exit (np_rerror (), "np_trans_write");
if (n != tc->size)
msg_exit ("np_trans_write came up unexpectedly short");
//msg ("sent tversion tag %d", tc->tag);
free(tc);
}
msg ("sent 100 tversions");

/* flush the most recent */
if (!(ac = np_create_tflush (flushtag)))
msg_exit ("out of memory");
tag = npc_get_id(fs->tagpool);
np_set_tag(ac, tag);
n = np_trans_write(fs->trans, ac->pkt, ac->size);
if (n < 0)
errn_exit (np_rerror (), "np_trans_write");
if (n != ac->size)
msg_exit ("np_trans_write came up unexpectedly short");
//msg ("sent tflush tag %d (flushing tag %d)", ac->tag, flushtag);
free (ac);
msg ("sent 1 tflush");

/* receive up to 101 responses with 1s timeout */
for (i = 0; i < 101; i++) {
if (!(rc = malloc(sizeof(*rc) + fs->msize)))
msg_exit ("out of memory");
rc->pkt = (u8*)rc + sizeof(*rc);
alarm (1);
n = np_trans_read(fs->trans, rc->pkt, fs->msize);
if (n < 0) {
if (errno == EINTR)
break;
errn_exit (np_rerror (), "np_trans_read");
}
alarm (0);
if (n == 0)
msg_exit ("np_trans_read: unexpected EOF");
if (!np_deserialize (rc, rc->pkt))
msg_exit ("failed to deserialize response in one go");
//msg ("received tag %d", rc->tag);
free(rc);
//npc_put_id(fs->tagpool, rc->tag);
}
if (i == 100 || i == 101)
msg ("received 100/101 respones");
else
msg ("received %d responses", i);
}

/*
* vi:tabstop=4 shiftwidth=4 expandtab
*/

0 comments on commit f3835a4

Please sign in to comment.