LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Neil Brown <neilb@suse.de>
To: "Jesper Juhl" <jesper.juhl@gmail.com>
Cc: nfs@lists.sourceforge.net,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Trond Myklebust <trond.myklebust@fys.uio.no>
Subject: Re: [NFS] 2.6.17.8 - do_vfs_lock: VFS is out of sync with lock manager!
Date: Mon, 21 Aug 2006 13:34:01 +1000	[thread overview]
Message-ID: <17641.10665.116168.867041@cse.unsw.edu.au> (raw)
In-Reply-To: message from Jesper Juhl on Thursday August 17

On Thursday August 17, jesper.juhl@gmail.com wrote:
> On 17/08/06, Neil Brown <neilb@suse.de> wrote:
> > On Tuesday August 8, jesper.juhl@gmail.com wrote:
> > > I have some webservers that have recently started reporting the
> > > following message in their logs :
> > >
> > >   do_vfs_lock: VFS is out of sync with lock manager!
> >
> >
> > I can imagine that happening if you mount with '-o nolocks'.
> > Then a non-blocking lock could cause that message (I think).
> > Can you conform that you aren't using 'nolocks'.
> >
> Confirmed.

Thanks.  I suspected as much but don't like to assume.

I've look more thoroughly at this code and I think the message is
meaningless and can be ignored.

Looking in fs/nfs/file.c (at 2.6.18-rc4-mm1 if it matters, but 2.6.17
is much the same)

 - do_vfs_lock is only called when the filesystem was mounted with
    -o nolock  EXCEPT
 - If a lock request to the server in interrupted (when mounted with
     -o intr) then do_vfs_lock is called to try to get the lock
    locally.  Normally equivalent code will be called inside
    fs/lockd/clntproc.c when the server replies that the lock has been
    gained.  In the case of an interrupt though this doesn't happen
    but the lock may still have happened on the server.  So we record
    locally that the lock was gained, to ensure that it gets unlocked
    when the process exits.

As you don't have '-o nolocks' you must be hitting the second case.
The lock call to the server returns -EINTR or -ERESTARTSYS and
do_vfs_lock is called just-in-case.  
As this is a just-in-case call, it is quite possible that the lock is
held by some other process, so getting an error is entirely possible.
So printing the message in this case seems wrong.

On the other hand, printing the message in any other case seems wrong
too, as server locking is not being used, so there is nothing to get
out of sync with.

As a further complication, I don't think that in the just-in-case
situation that it should risk waiting for the lock.
Now maybe we can be sure there is a pending signal which will break
out of any wait (though I'm worried about -ERESTARTSYS - that doesn't
imply a signal does it?), but I would feel more comfortable if
FL_SLEEP were turned off in that path.

So: Trond:  Any obvious errors in the above?
Is the following patch ok?

NeilBrown


Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/nfs/file.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff .prev/fs/nfs/file.c ./fs/nfs/file.c
--- .prev/fs/nfs/file.c	2006-08-21 13:28:25.000000000 +1000
+++ ./fs/nfs/file.c	2006-08-21 13:30:27.000000000 +1000
@@ -452,9 +452,6 @@ static int do_vfs_lock(struct file *file
 		default:
 			BUG();
 	}
-	if (res < 0)
-		printk(KERN_WARNING "%s: VFS is out of sync with lock manager!\n",
-				__FUNCTION__);
 	return res;
 }
 
@@ -504,10 +501,13 @@ static int do_setlk(struct file *filp, i
 		 * we clean up any state on the server. We therefore
 		 * record the lock call as having succeeded in order to
 		 * ensure that locks_remove_posix() cleans it out when
-		 * the process exits.
+		 * the process exits.  Make sure not to sleep if
+		 * someone else holds the lock.
 		 */
-		if (status == -EINTR || status == -ERESTARTSYS)
+		if (status == -EINTR || status == -ERESTARTSYS) {
+			fl->fl_flags &= ~FL_SLEEP;
 			do_vfs_lock(filp, fl);
+		}
 	} else
 		status = do_vfs_lock(filp, fl);
 	unlock_kernel();

 
  

  reply	other threads:[~2006-08-21  3:34 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-08 14:39 Jesper Juhl
2006-08-09  5:53 ` Grant Coady
2006-08-09  8:07   ` Jesper Juhl
2006-08-10 22:37     ` Jesper Juhl
2006-08-11  0:30       ` Grant Coady
2006-08-13 23:08       ` Grant Coady
2006-08-17  6:49 ` [NFS] " Neil Brown
2006-08-17  9:58   ` Jesper Juhl
2006-08-21  3:34     ` Neil Brown [this message]
2006-08-21 19:54       ` Trond Myklebust
2006-11-21 12:43         ` Jesper Juhl
2006-11-27  9:19           ` Jesper Juhl
2007-01-29  5:08         ` Neil Brown
2007-01-29 14:16           ` Trond Myklebust
2007-01-30 23:42             ` Jesper Juhl
2007-02-01 22:39               ` Neil Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=17641.10665.116168.867041@cse.unsw.edu.au \
    --to=neilb@suse.de \
    --cc=jesper.juhl@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nfs@lists.sourceforge.net \
    --cc=trond.myklebust@fys.uio.no \
    --subject='Re: [NFS] 2.6.17.8 - do_vfs_lock: VFS is out of sync with lock manager'\!'' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).