From owner-svn-src-all@FreeBSD.ORG Sun Sep 22 20:14:23 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 23455433; Sun, 22 Sep 2013 20:14:23 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-ob0-x22b.google.com (mail-ob0-x22b.google.com [IPv6:2607:f8b0:4003:c01::22b]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id AC0F62413; Sun, 22 Sep 2013 20:14:22 +0000 (UTC) Received: by mail-ob0-f171.google.com with SMTP id wm4so2883946obc.16 for ; Sun, 22 Sep 2013 13:14:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=ylXmO4s8HOAoWMvKqQWKTQ2aTaW9oNW1YGTmKsdeRwE=; b=Ofm6YIiA7hasgu7SnxZ8K80iHu42oUVSv6QFZza67USJ/cnglkl4/vTsSzPgn9YKoW 2/ybIrAX6Kmdtg+UNXLOdq9dO4atrdcSZAYd1G4HsPOgfZH8QGCR2AaNhQV4pzKPJ2+0 wL8O0+gK4w4GGocUgNQor/6VtpmAL2ECF4j/iX8muMmScxMVw5fXHtJC+GOz+u5yFP/n aPXZsbpR8Iczt67dyZ10Ng4eGucgm8QMsy5UxXoRrXgSVpWpYmXjsinnOkYi11jxRqFM e1tV9ifoKohoUqDMD7zWsp2jyu588aRSTlFGJH8SqwimXarvkC02MaIL0fIM1ab3cbkC zxNw== MIME-Version: 1.0 X-Received: by 10.182.130.131 with SMTP id oe3mr216643obb.34.1379880861963; Sun, 22 Sep 2013 13:14:21 -0700 (PDT) Sender: mdf356@gmail.com Received: by 10.182.75.9 with HTTP; Sun, 22 Sep 2013 13:14:21 -0700 (PDT) In-Reply-To: <201309221923.r8MJNm3u021657@svn.freebsd.org> References: <201309221923.r8MJNm3u021657@svn.freebsd.org> Date: Sun, 22 Sep 2013 13:14:21 -0700 X-Google-Sender-Auth: uvyHNtFyWm0SgWpQYsFfGz4Ds2k Message-ID: Subject: Re: svn commit: r255797 - head/sys/kern From: Matthew Fleming To: Konstantin Belousov Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: "svn-src-head@freebsd.org" , Attilio Rao , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 22 Sep 2013 20:14:23 -0000 On Sun, Sep 22, 2013 at 12:23 PM, Konstantin Belousov wrote: > Author: kib > Date: Sun Sep 22 19:23:48 2013 > New Revision: 255797 > URL: http://svnweb.freebsd.org/changeset/base/255797 > > Log: > Increase the chance of the buffer write from the bufdaemon helper > context to succeed. If the locked vnode which owns the buffer to be > written is shared locked, try the non-blocking upgrade of the lock to > exclusive. > > PR: kern/178997 > Reported and tested by: Klaus Weber < > fbsd-bugs-2013-1@unix-admin.de> > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > Approved by: re (marius) > > Modified: > head/sys/kern/vfs_bio.c > > Modified: head/sys/kern/vfs_bio.c > > ============================================================================== > --- head/sys/kern/vfs_bio.c Sun Sep 22 19:15:24 2013 (r255796) > +++ head/sys/kern/vfs_bio.c Sun Sep 22 19:23:48 2013 (r255797) > @@ -2624,6 +2624,8 @@ flushbufqueues(struct vnode *lvp, int ta > int hasdeps; > int flushed; > int queue; > + int error; > + bool unlock; > > flushed = 0; > queue = QUEUE_DIRTY; > @@ -2699,7 +2701,16 @@ flushbufqueues(struct vnode *lvp, int ta > BUF_UNLOCK(bp); > continue; > } > - if (vn_lock(vp, LK_EXCLUSIVE | LK_NOWAIT | LK_CANRECURSE) > == 0) { > + if (lvp == NULL) { > + unlock = true; > + error = vn_lock(vp, LK_EXCLUSIVE | LK_NOWAIT); > + } else { > + ASSERT_VOP_LOCKED(vp, "getbuf"); > + unlock = false; > + error = VOP_ISLOCKED(vp) == LK_EXCLUSIVE ? 0 : > + vn_lock(vp, LK_UPGRADE | LK_NOWAIT); > I don't think this is quite right. When the lock is held shared, and VOP_LOCK is implemented by lockmgr(9), (i.e. all in-tree filesystems?), LK_UPGRADE may drop the lock, and not reacquire it. This would happen when the vnode is locked shared, the upgrade fails (2 shared owners), then lockmgr(9) will try to lock EX, which will also fail (still one shared owner). The caller's lock is no longer held. Doesn't that scenario (LK_UPGRADE failing) cause problems both for the caller (unexpected unlock) and for flushbufqueues(), which expects the vnode lock to be held since lvp is non-NULL? Thanks, matthew