From mboxrd@z Thu Jan  1 00:00:00 1970
From: Matthew Wilcox <willy@debian.org>
Subject: Re: lockd's interactions with locks.c
Date: Fri, 2 Aug 2002 18:21:03 +0100
Sender: linux-fsdevel-owner@vger.kernel.org
Message-ID: <20020802182103.D24631@parcelfarce.linux.theplanet.co.uk>
References: <20020801022821.E3797@parcelfarce.linux.theplanet.co.uk> <shsfzxxckzi.fsf@charged.uio.no>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Matthew Wilcox <willy@debian.org>, linux-fsdevel@vger.kernel.org,
	neilb@cse.unsw.edu.au, okir@monad.swb.de
Return-path: <linux-fsdevel-owner@vger.kernel.org>
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Content-Disposition: inline
In-Reply-To: <shsfzxxckzi.fsf@charged.uio.no>; from trond.myklebust@fys.uio.no on Fri, Aug 02, 2002 at 05:56:49PM +0200
List-Id: linux-fsdevel.vger.kernel.org


[nfs-devel trimmed from recipient list as it appears to be an unmaintained
email alias]

On Fri, Aug 02, 2002 at 05:56:49PM +0200, Trond Myklebust wrote:
> I'm not sure I understand how are you are planning on protecting
> against races with the blocking code? For instance
> 
>      lockd:                                 Another process:
> 
>                                             posix_lock_file();
>    posix_lock_file();
> 
>                                            releases file lock;
> 
>   grab_lockd_blocking_lock();
>   nlmsvc_insert_block();
>   have_been_woken_up_already = ...;
>   release_lockd_blocking_lock();
> 
> 
> Is this a situation where the mysterious 'have_been_woken_up_already'
> kicks in in order to tell lockd not to block after all? If so, how do
> you see that part being implemented?

Yes, that's exactly where it kicks in.  I did some hacking on the plane
yesterday and now I think I understand how it should work.  Also, I now
understand that the model I had in mind for posix_lock_file() cannot
work for lockd without a major overhaul (and I don't particularly want
to start hacking on lockd).  So consider the flock-A patch I pointed at
as nothing more than a proof-of-concept.  Anyway... here's what I have:

u32 nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
			struct nlm_lock *lock, int wait,
			struct nlm_cookie *cookie)
{
	struct file_lock	*conflock;
	struct nlm_block	*block;
	int			error;

	dprintk("lockd: nlmsvc_lock(%s/%ld, ty=%d, pi=%d, %Ld-%Ld, bl=%d)\n",
				file->f_file.f_dentry->d_inode->i_sb->s_id,
				file->f_file.f_dentry->d_inode->i_ino,
				lock->fl.fl_type, lock->fl.fl_pid,
				(long long)lock->fl.fl_start,
				(long long)lock->fl.fl_end,
				wait);

	/* Lock file against concurrent access */
	down(&file->f_sema);

	/* Get existing block (in case client is busy-waiting) */
	block = nlmsvc_lookup_block(file, lock, 0);

	lock->fl.fl_flags |= FL_LOCKD;
	if (wait)
		lock->fl.fl_flags |= FL_SLEEP;

	error = posix_lock_file(&file->f_file, &lock->fl);
	if (!error)
		goto out;

	if (!wait || (error != -EAGAIN))
		goto out;

	/* If we don't have a block, create and initialize it. */
	if (!block) {
		dprintk("lockd: blocking on this lock (allocating).\n");
		block = nlmsvc_create_block(rqstp, file, lock, cookie);
		error = -ENOLCK
		if (!block)
			goto out;
	}

	/* Append to list of blocked */
	nlmsvc_insert_block(block, NLM_NEVER);

	/* A wakeup may have come in between returning from posix_lock_file
	 * and nlmsvc_insert_block.  If it has, we have to move the block
	 * to the head of the list and kick lockd to retry the lock.  If a
	 * wakeup comes in between insert_block and the test, we wake up
	 * the daemon twice.  No big deal.
	 */
	if (!block->b_call.a_args.lock.fl.fl_next) {
		nlmsvc_insert_block(block, 0);
		svc_wake_up(block->b_daemon);
	}

	up(&file->f_sema);
	return nlm_lck_blocked;

 out:
	if (block)
		nlmsvc_delete_block(block, 0);
 unlock:
	up(&file->f_sema);
	switch (error) {
	case 0:
		return nlm_granted;
	case -EDEADLK:
		return nlm_deadlock;
	case -EAGAIN:
		return nlm_lck_denied;
	case -ENOLCK:
		return nlm_lck_denied_nolocks;
	}
}

Note I haven't even tried compiling this yet.

-- 
Revolutions do not require corporate support.