From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Howells Subject: Re: [PATCH] AFS: Implement file locking Date: Sun, 27 May 2007 00:55:30 +0100 Message-ID: <7436.1180223730@redhat.com> References: <20070526022342.GA20905@fieldses.org> <20070524165554.22292.38887.stgit@warthog.cambridge.redhat.com> Cc: akpm@osdl.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: "J. Bruce Fields" Return-path: Received: from mx1.redhat.com ([66.187.233.31]:36884 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751592AbXEZXzi (ORCPT ); Sat, 26 May 2007 19:55:38 -0400 In-Reply-To: <20070526022342.GA20905@fieldses.org> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org J. Bruce Fields wrote: > > + if (!afs_lock_manager) { > > + afs_lock_manager = create_singlethread_workqueue("kafs_lockd"); > > + if (!afs_lock_manager) > > + return -ENOMEM; > > + } > > + return 0; > > Doesn't this need some locking? Oops. Yes. It used to be inside the lock_kernel() section, but has since escaped. > Do you allow upgrades and downgrades? (Just curious.) AFS does not, as far as I know. Upgrades are dangerous anyway as you can get deadlock quite easily. > > + /* if we've already got a readlock on the server and no waiting > > + * writelocks, then we might be able to instantly grant another > > Is that comment correct? (You don't really test for "waiting > writelocks", do you?) Locally, yes. 'if (list_empty(&vnode->pending_locks))' covers it quite handily. I can't do anything about checking the server. David