From patchwork Sun Apr 12 18:49:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Van Hensbergen X-Patchwork-Id: 6203911 Return-Path: X-Original-To: patchwork-v9fs-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1CE9D9F2EC for ; Sun, 12 Apr 2015 18:50:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0AB5E200F3 for ; Sun, 12 Apr 2015 18:50:20 +0000 (UTC) Received: from lists.sourceforge.net (lists.sourceforge.net [216.34.181.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 66F422008F for ; Sun, 12 Apr 2015 18:50:18 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=sfs-ml-4.v29.ch3.sourceforge.com) by sfs-ml-4.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1YhMxT-0002pm-LL; Sun, 12 Apr 2015 18:50:15 +0000 Received: from sog-mx-1.v43.ch3.sourceforge.com ([172.29.43.191] helo=mx.sourceforge.net) by sfs-ml-4.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1YhMxS-0002pg-Hq for v9fs-developer@lists.sourceforge.net; Sun, 12 Apr 2015 18:50:14 +0000 Received-SPF: pass (sog-mx-1.v43.ch3.sourceforge.com: domain of gmail.com designates 209.85.220.176 as permitted sender) client-ip=209.85.220.176; envelope-from=ericvh@gmail.com; helo=mail-qk0-f176.google.com; Received: from mail-qk0-f176.google.com ([209.85.220.176]) by sog-mx-1.v43.ch3.sourceforge.com with esmtps (TLSv1:RC4-SHA:128) (Exim 4.76) id 1YhMxQ-0004MS-NS for v9fs-developer@lists.sourceforge.net; Sun, 12 Apr 2015 18:50:14 +0000 Received: by qkx62 with SMTP id 62so138140843qkx.0 for ; Sun, 12 Apr 2015 11:50:07 -0700 (PDT) X-Received: by 10.182.27.103 with SMTP id s7mr9756920obg.2.1428864606400; Sun, 12 Apr 2015 11:50:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.202.197.135 with HTTP; Sun, 12 Apr 2015 11:49:46 -0700 (PDT) From: Eric Van Hensbergen Date: Sun, 12 Apr 2015 13:49:46 -0500 Message-ID: To: V9FS Developers X-Spam-Score: -0.6 (/) X-Headers-End: 1YhMxQ-0004MS-NS X-Content-Filtered-By: Mailman/MimeDel 2.1.9 Subject: [V9fs-developer] [RFC PATCH]: 9p create-unlink-fstat fix X-BeenThere: v9fs-developer@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: v9fs-developer-bounces@lists.sourceforge.net X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Here's a swag at a patch to address the open-unlink-fstat issue that's been reported, mostly by folks using virtio-9p but it seems to happen with diod as well. Some details on it here for those interested in reading more: https://bugs.launchpad.net/qemu/+bug/1336794 Its fairly nasty and seems better addressed by changing the VFS API so we don't need to guess. I'm fairly certain I'm probably doing something wrong in this patch (probably by adding open fids to the dentry list and not removing them anywhere explicitly). Any other solutions to this problem are welcome.... Signed-off-by: Eric Van Hensbegren --- fs/9p/fid.c | 63 +++++++++++++++++++++++++++++++++++--------------- fs/9p/fid.h | 7 ++++++ fs/9p/vfs_dentry.c | 8 +++++-- fs/9p/vfs_inode_dotl.c | 12 +++++++--- 4 files changed, 67 insertions(+), 23 deletions(-) if (IS_ERR(fid)) { @@ -484,9 +487,12 @@ v9fs_vfs_getattr_dotl(struct vfsmount *mnt, struct dentry *dentry, generic_fillattr(dentry->d_inode, stat); return 0; } - fid = v9fs_fid_lookup(dentry); - if (IS_ERR(fid)) - return PTR_ERR(fid); + fid = v9fs_fid_lookup_flags(dentry, FID_OPEN); + if (IS_ERR(fid)) { + fid = v9fs_fid_lookup(dentry); + if (IS_ERR(fid)) + return PTR_ERR(fid); + } /* Ask for all the fields in stat structure. Server will return * whatever it supports diff --git a/fs/9p/fid.c b/fs/9p/fid.c index 47db55a..d480554 100644 --- a/fs/9p/fid.c +++ b/fs/9p/fid.c @@ -57,26 +57,36 @@ void v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid) * v9fs_fid_find - retrieve a fid that belongs to the specified uid * @dentry: dentry to look for fid in * @uid: return fid that belongs to the specified user - * @any: if non-zero, return any fid associated with the dentry + * @flags: modifiers for fid lookup * */ - -static struct p9_fid *v9fs_fid_find(struct dentry *dentry, kuid_t uid, int any) +static struct p9_fid * +v9fs_fid_find(struct dentry *dentry, kuid_t uid, int flags) { struct p9_fid *fid, *ret; - p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p) uid %d any %d\n", + p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p) uid %d flags %d\n", dentry, dentry, from_kuid(&init_user_ns, uid), - any); + flags); ret = NULL; /* we'll recheck under lock if there's anything to look in */ if (dentry->d_fsdata) { struct hlist_head *h = (struct hlist_head *)&dentry->d_fsdata; spin_lock(&dentry->d_lock); hlist_for_each_entry(fid, h, dlist) { - if (any || uid_eq(fid->uid, uid)) { - ret = fid; - break; + if ((flags & FID_ANY) || uid_eq(fid->uid, uid)) { + if (flags & FID_OPEN) { + p9_debug(P9_DEBUG_9P, + " fid: %d fid->mode: %d\n", fid->fid, + fid->mode); + if (fid->mode != -1) { + ret = fid; + break; + } + } else { + ret = fid; + break; + } } } spin_unlock(&dentry->d_lock); @@ -114,7 +124,7 @@ err_out: } static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, - kuid_t uid, int any) + kuid_t uid, int flags) { struct dentry *ds; char **wnames, *uname; @@ -124,9 +134,13 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, v9ses = v9fs_dentry2v9ses(dentry); access = v9ses->flags & V9FS_ACCESS_MASK; - fid = v9fs_fid_find(dentry, uid, any); + fid = v9fs_fid_find(dentry, uid, flags); if (fid) return fid; + + if (flags & FID_OPEN) + return ERR_PTR(-ENOENT); + /* * we don't have a matching fid. To do a TWALK we need * parent fid. We need to prevent rename when we want to @@ -134,7 +148,7 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, */ down_read(&v9ses->rename_sem); ds = dentry->d_parent; - fid = v9fs_fid_find(ds, uid, any); + fid = v9fs_fid_find(ds, uid, flags); if (fid) { /* Found the parent fid do a lookup with that */ fid = p9_client_walk(fid, 1, (char **)&dentry->d_name.name, 1); @@ -143,7 +157,7 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, up_read(&v9ses->rename_sem); /* start from the root and try to do a lookup */ - fid = v9fs_fid_find(dentry->d_sb->s_root, uid, any); + fid = v9fs_fid_find(dentry->d_sb->s_root, uid, flags); if (!fid) { /* the user is not attached to the fs yet */ if (access == V9FS_ACCESS_SINGLE) @@ -227,11 +241,26 @@ err_out: * dentry (if it has one), or the root dentry. If the user haven't accessed * the fs yet, attach now and walk from the root. */ - struct p9_fid *v9fs_fid_lookup(struct dentry *dentry) { + return v9fs_fid_lookup_flags(dentry, 0); +} + +/** + * v9fs_fid_lookup_flags - lookup for a fid w/flags, try to walk if not found + * @dentry: dentry to look for fid in + * @flags: potential flags + * FID_OPEN: favor open fids + * + * Look for a fid in the specified dentry for the current user. + * If no fid is found, try to create one walking from a fid from the parent + * dentry (if it has one), or the root dentry. If the user haven't accessed + * the fs yet, attach now and walk from the root. + */ +struct p9_fid *v9fs_fid_lookup_flags(struct dentry *dentry, int flags) +{ kuid_t uid; - int any, access; + int access; struct v9fs_session_info *v9ses; v9ses = v9fs_dentry2v9ses(dentry); @@ -241,20 +270,18 @@ struct p9_fid *v9fs_fid_lookup(struct dentry *dentry) case V9FS_ACCESS_USER: case V9FS_ACCESS_CLIENT: uid = current_fsuid(); - any = 0; break; case V9FS_ACCESS_ANY: uid = v9ses->uid; - any = 1; + flags |= FID_ANY; break; default: uid = INVALID_UID; - any = 0; break; } - return v9fs_fid_lookup_with_uid(dentry, uid, any); + return v9fs_fid_lookup_with_uid(dentry, uid, flags); } struct p9_fid *v9fs_fid_clone(struct dentry *dentry) diff --git a/fs/9p/fid.h b/fs/9p/fid.h index 2b6787f..e631c0f 100644 --- a/fs/9p/fid.h +++ b/fs/9p/fid.h @@ -23,7 +23,14 @@ #define FS_9P_FID_H #include +enum p9_fid_flags { + FID_NONE = 0x0, + FID_ANY = 0x1, + FID_OPEN = 0x2, +}; + struct p9_fid *v9fs_fid_lookup(struct dentry *dentry); +struct p9_fid *v9fs_fid_lookup_flags(struct dentry *dentry, int flags); struct p9_fid *v9fs_fid_clone(struct dentry *dentry); void v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid); struct p9_fid *v9fs_writeback_fid(struct dentry *dentry); diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c index a345b2d..577815b 100644 --- a/fs/9p/vfs_dentry.c +++ b/fs/9p/vfs_dentry.c @@ -69,8 +69,12 @@ static void v9fs_dentry_release(struct dentry *dentry) struct hlist_node *p, *n; p9_debug(P9_DEBUG_VFS, " dentry: %pd (%p)\n", dentry, dentry); - hlist_for_each_safe(p, n, (struct hlist_head *)&dentry->d_fsdata) - p9_client_clunk(hlist_entry(p, struct p9_fid, dlist)); + hlist_for_each_safe(p, n, (struct hlist_head *)&dentry->d_fsdata) { + struct p9_fid *fid = hlist_entry(p, struct p9_fid, dlist); + /* only clunk unopen fids when cleaning up dentry */ + if (fid->mode == -1) + p9_client_clunk(fid); + } dentry->d_fsdata = NULL; } diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c index 6054c16b..2f17675 100644 --- a/fs/9p/vfs_inode_dotl.c +++ b/fs/9p/vfs_inode_dotl.c @@ -308,6 +308,9 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, } v9fs_invalidate_inode_attr(dir); + /* add the open FID to the dentry */ + v9fs_fid_add(dentry, ofid); + /* instantiate inode and assign the unopened fid to the dentry */ fid = p9_client_walk(dfid, 1, &name, 1);