From patchwork Thu Oct 14 12:24:38 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arun Bharadwaj X-Patchwork-Id: 67815 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id CF01EB70F5 for ; Thu, 14 Oct 2010 23:31:43 +1100 (EST) Received: from localhost ([127.0.0.1]:54439 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P6My3-0004aP-Ot for incoming@patchwork.ozlabs.org; Thu, 14 Oct 2010 08:31:31 -0400 Received: from [140.186.70.92] (port=52173 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P6Mra-0007t8-7l for qemu-devel@nongnu.org; Thu, 14 Oct 2010 08:24:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P6MrY-0003ZG-NS for qemu-devel@nongnu.org; Thu, 14 Oct 2010 08:24:50 -0400 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:35620) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P6MrX-0003Yv-Ej for qemu-devel@nongnu.org; Thu, 14 Oct 2010 08:24:48 -0400 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by e28smtp06.in.ibm.com (8.14.4/8.13.1) with ESMTP id o9ECOkPC026685 for ; Thu, 14 Oct 2010 17:54:46 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o9ECOjPI3690608 for ; Thu, 14 Oct 2010 17:54:45 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o9ECOjeE024639 for ; Thu, 14 Oct 2010 23:24:45 +1100 Received: from localhost6.localdomain6 ([9.77.206.195]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id o9ECOfGn024326 for ; Thu, 14 Oct 2010 23:24:43 +1100 To: qemu-devel@nongnu.org From: Arun R Bharadwaj Date: Thu, 14 Oct 2010 17:54:38 +0530 Message-ID: <20101014122438.2238.93872.stgit@localhost6.localdomain6> In-Reply-To: <20101014122217.2238.94995.stgit@localhost6.localdomain6> References: <20101014122217.2238.94995.stgit@localhost6.localdomain6> User-Agent: StGit/0.15 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) Subject: [Qemu-devel] [PATCH 5/9] Convert wstat into 2nd threading model X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Sripathi Kodi In this model we hand over the vcpu thread only executes till the first blocking operation. It then hands over the call to the worker thread, which does everything needed to complete the call. It can make multiple blocking calls. It finally signals the IO thread to do complete_pdu(). Simplification of code due to this thread model is obvious in this patch. Gautham suggested that I use a generic function to call complete_pdu, which could further simplify the post_op structure. However, some of the calls (stat included) need to free up some memory after calling complete_pdu. Handling this becomes messy if I use a common complete function. Review comments welcome. Review comment from Gautham is a necessity. Signed-off-by: Sripathi Kodi --- hw/virtio-9p.c | 238 +++++++++++++++++++++----------------------------------- hw/virtio-9p.h | 4 + 2 files changed, 94 insertions(+), 148 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index d994293..1f2fd9f 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -2873,45 +2873,13 @@ out: qemu_free(vs); } -static void v9fs_wstat_post_truncate(V9fsState *s, V9fsWstatState *vs, int err) -{ - if (err < 0) { - goto out; - } - - err = vs->offset; - -out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} - -static void v9fs_wstat_post_rename(V9fsState *s, V9fsWstatState *vs, int err) -{ - if (err < 0) { - goto out; - } - if (vs->v9stat.length != -1) { - if (v9fs_do_truncate(s, &vs->fidp->path, vs->v9stat.length) < 0) { - err = -errno; - } - } - v9fs_wstat_post_truncate(s, vs, err); - return; - -out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} - static int v9fs_complete_rename(V9fsState *s, V9fsRenameState *vs) { int err = 0; char *old_name, *new_name; char *end; + qemu_rwmutex_wrlock(&global_rename_lock); if (vs->newdirfid != -1) { V9fsFidState *dirfidp; dirfidp = lookup_fid(s, vs->newdirfid); @@ -2955,6 +2923,9 @@ static int v9fs_complete_rename(V9fsState *s, V9fsRenameState *vs) * Fixup fid's pointing to the old name to * start pointing to the new name */ + /* FIXME: Since we are holding the global rename + * lock in write mode there is no need to hold fid_list_lock + */ qemu_rwmutex_rdlock(&s->fid_list_lock); for (fidp = s->fid_list; fidp; fidp = fidp->next) { if (vs->fidp == fidp) { @@ -2976,41 +2947,14 @@ static int v9fs_complete_rename(V9fsState *s, V9fsRenameState *vs) } } out: + qemu_rwmutex_unlock(&global_rename_lock); v9fs_string_free(&vs->name); return err; } static void v9fs_rename_post_rename(V9fsState *s, V9fsRenameState *vs, int err) { - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} - -static void v9fs_wstat_post_chown(V9fsState *s, V9fsWstatState *vs, int err) -{ - if (err < 0) { - goto out; - } - - if (vs->v9stat.name.size != 0) { - V9fsRenameState *vr; - - vr = qemu_mallocz(sizeof(V9fsRenameState)); - vr->newdirfid = -1; - vr->pdu = vs->pdu; - vr->fidp = vs->fidp; - vr->offset = vs->offset; - vr->name.size = vs->v9stat.name.size; - vr->name.data = qemu_strdup(vs->v9stat.name.data); - - err = v9fs_complete_rename(s, vr); - qemu_free(vr); - } - v9fs_wstat_post_rename(s, vs, err); - return; - -out: - v9fs_stat_free(&vs->v9stat); +/* FIXME: Needed? v9fs_stat_free(&vs->v9stat); */ complete_pdu(s, vs->pdu, err); qemu_free(vs); } @@ -3043,31 +2987,55 @@ out: qemu_free(vs); } -static void v9fs_wstat_post_utime(V9fsState *s, V9fsWstatState *vs, int err) +static void v9fs_wstat_do_complete(void *opaque) { - if (err < 0) { + V9fsWstatState *vs = (V9fsWstatState *)opaque; + complete_pdu(vs->s, vs->pdu, vs->err); + v9fs_stat_free(&vs->v9stat); + qemu_free(vs); +} + +static void v9fs_wstat_worker(ThreadletWork *work) +{ + uint32_t v9_mode; + V9fsWstatState *vs = container_of(work, V9fsWstatState, work); + + vs->fidp = lookup_fid(vs->s, vs->fid); + if (vs->fidp == NULL) { + vs->err = -EINVAL; goto out; } - if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) { - if (v9fs_do_chown(s, &vs->fidp->path, vs->v9stat.n_uid, - vs->v9stat.n_gid)) { - err = -errno; + /* do we need to sync the file? */ + if (donttouch_stat(&vs->v9stat)) { + vs->err = v9fs_do_fsync(vs->s, vs->fidp->fs.fd); + if (vs->err == -1) { + vs->err = -errno; + goto out; } } - v9fs_wstat_post_chown(s, vs, err); - return; -out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} + if (vs->v9stat.mode != -1) { + qemu_rwmutex_rdlock(&global_rename_lock); + vs->err = v9fs_do_lstat(vs->s, &vs->fidp->path, &vs->stbuf); + if (vs->err == -1) { + vs->err = -errno; + goto out_unlock; + } + v9_mode = stat_to_v9mode(&vs->stbuf); -static void v9fs_wstat_post_chmod(V9fsState *s, V9fsWstatState *vs, int err) -{ - if (err < 0) { - goto out; + if ((vs->v9stat.mode & P9_STAT_MODE_TYPE_BITS) != + (v9_mode & P9_STAT_MODE_TYPE_BITS)) { + /* Attempting to change the type */ + vs->err = -EIO; + goto out_unlock; + } + if (v9fs_do_chmod(vs->s, &vs->fidp->path, v9mode_to_mode(vs->v9stat.mode, + &vs->v9stat.extension))) { + vs->err = -errno; + goto out_unlock; + } + qemu_rwmutex_unlock(&global_rename_lock); } if (vs->v9stat.mtime != -1 || vs->v9stat.atime != -1) { @@ -3085,99 +3053,73 @@ static void v9fs_wstat_post_chmod(V9fsState *s, V9fsWstatState *vs, int err) times[1].tv_nsec = UTIME_OMIT; } - if (v9fs_do_utimensat(s, &vs->fidp->path, times)) { - err = -errno; + qemu_rwmutex_rdlock(&global_rename_lock); + if (v9fs_do_utimensat(vs->s, &vs->fidp->path, times)) { + vs->err = -errno; + goto out_unlock; } + qemu_rwmutex_unlock(&global_rename_lock); } - v9fs_wstat_post_utime(s, vs, err); - return; - -out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} - -static void v9fs_wstat_post_fsync(V9fsState *s, V9fsWstatState *vs, int err) -{ - if (err == -1) { - err = -errno; + if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) { + qemu_rwmutex_rdlock(&global_rename_lock); + if (v9fs_do_chown(vs->s, &vs->fidp->path, vs->v9stat.n_uid, + vs->v9stat.n_gid)) { + vs->err = -errno; + goto out_unlock; + } + qemu_rwmutex_unlock(&global_rename_lock); } - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); -} -static void v9fs_wstat_post_lstat(V9fsState *s, V9fsWstatState *vs, int err) -{ - uint32_t v9_mode; - - if (err == -1) { - err = -errno; - goto out; - } + if (vs->v9stat.name.size != 0) { + V9fsRenameState *vr; - v9_mode = stat_to_v9mode(&vs->stbuf); + vr = qemu_mallocz(sizeof(V9fsRenameState)); + vr->newdirfid= -1; + vr->pdu = vs->pdu; + vr->fidp = vs->fidp; + vr->offset = vs->offset; + vr->name.size = vs->v9stat.name.size; + vr->name.data = qemu_strdup(vs->v9stat.name.data); - if ((vs->v9stat.mode & P9_STAT_MODE_TYPE_BITS) != - (v9_mode & P9_STAT_MODE_TYPE_BITS)) { - /* Attempting to change the type */ - err = -EIO; + vs->err = v9fs_complete_rename(vs->s, vr); + qemu_free(vr); + if (vs->err < 0) { goto out; + } } - if (v9fs_do_chmod(s, &vs->fidp->path, v9mode_to_mode(vs->v9stat.mode, - &vs->v9stat.extension))) { - err = -errno; - } - v9fs_wstat_post_chmod(s, vs, err); - return; + if (vs->v9stat.length != -1) { + qemu_rwmutex_rdlock(&global_rename_lock); + if (v9fs_do_truncate(vs->s, &vs->fidp->path, vs->v9stat.length) < 0) { + vs->err = -errno; + goto out_unlock; + } + qemu_rwmutex_unlock(&global_rename_lock); + } + + goto out; +out_unlock: + qemu_rwmutex_unlock(&global_rename_lock); out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); + v9fs_async_helper_done(v9fs_wstat_do_complete, vs); } static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu) { - int32_t fid; V9fsWstatState *vs; - int err = 0; vs = qemu_malloc(sizeof(*vs)); vs->pdu = pdu; vs->offset = 7; + vs->s = s; - pdu_unmarshal(pdu, vs->offset, "dwS", &fid, &vs->unused, &vs->v9stat); - - vs->fidp = lookup_fid(s, fid); - if (vs->fidp == NULL) { - err = -EINVAL; - goto out; - } - - /* do we need to sync the file? */ - if (donttouch_stat(&vs->v9stat)) { - err = v9fs_do_fsync(s, vs->fidp->fs.fd); - v9fs_wstat_post_fsync(s, vs, err); - return; - } + pdu_unmarshal(pdu, vs->offset, "dwS", &vs->fid, &vs->unused, &vs->v9stat); - if (vs->v9stat.mode != -1) { - err = v9fs_do_lstat(s, &vs->fidp->path, &vs->stbuf); - v9fs_wstat_post_lstat(s, vs, err); - return; - } - - v9fs_wstat_post_chmod(s, vs, err); + vs->work.func = v9fs_wstat_worker; + submit_threadlet(&vs->work); return; - -out: - v9fs_stat_free(&vs->v9stat); - complete_pdu(s, vs->pdu, err); - qemu_free(vs); } static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err) diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h index 9ced508..59f7a4e 100644 --- a/hw/virtio-9p.h +++ b/hw/virtio-9p.h @@ -354,6 +354,10 @@ typedef struct V9fsWstatState V9fsStat v9stat; V9fsFidState *fidp; struct stat stbuf; + V9fsState *s; + int32_t fid; + int32_t err; + ThreadletWork work; } V9fsWstatState; typedef struct V9fsSymlinkState