All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Teigland <teigland@redhat.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Jeff Layton <jlayton@redhat.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] lockd: handle fl_grant callbacks with coalesced locks (RFC)
Date: Thu, 15 Jan 2009 10:30:14 -0600	[thread overview]
Message-ID: <20090115163014.GA6602@redhat.com> (raw)
In-Reply-To: <20081217200156.GO4614@fieldses.org>

On Wed, Dec 17, 2008 at 03:01:56PM -0500, J. Bruce Fields wrote:
> On Wed, Dec 17, 2008 at 01:14:53PM -0600, David Teigland wrote:
> > Jeff suggested the following patch, which I've tried and it fixes the
> > problem I was seeing.  It passes the original, unmodified file_lock to
> > notify(), instead of the copy which is passed to (and coalesced by)
> > posix_lock_file().  I'm guessing this was reason for having a copy of the
> > file_lock in the first place, but it was just not used correctly.
> 
> Yep, that looks much better.  Though actually I suspect what was really
> intended was to use "flc" for the notifies, and "fl" for the
> posix_lock_file().
> 
> Also, since flc is never actually handed to the posix lock system, I
> think it should be a "shallow" lock copy--so it should be created with
> __locks_copy_lock().  Something like the below?

I left this hanging over the holidays and I'd like to get it wrapped up.
In summary, I think the following is the correct fix:
  http://marc.info/?l=linux-nfs&m=122954145532438&w=2

(I'd like to do the s/locks_copy_lock/__locks_copy_lock/ in a separate
patch since it's not directly related to fixing the bug.)

Bruce suggested that perhaps my patch should swap "fl" and "flc", which
I don't think is correct (and doesn't fix the problem in a test).  Here's
my complicated explanation of that:
  http://marc.info/?l=linux-nfs&m=122954948914263&w=2

Without any further feedback, I'll plan to send my patch soon for 2.6.29.
Thanks,
Dave


  parent reply	other threads:[~2009-01-15 16:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-19 21:37 [PATCH] lockd: handle fl_grant callbacks with coalesced locks (RFC) Jeff Layton
2008-11-22  1:15 ` J. Bruce Fields
2008-11-24 15:33   ` Jeff Layton
     [not found]     ` <20081124103313.0c779324-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-11-24 17:06       ` J. Bruce Fields
2008-11-25 15:12         ` Jeff Layton
2008-12-13 12:40         ` Jeff Layton
     [not found]           ` <20081213074042.2e8223c3-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-12-16 19:38             ` J. Bruce Fields
2008-12-16 19:56               ` J. Bruce Fields
2008-12-16 21:11                 ` Jeff Layton
     [not found]                   ` <20081216161158.2d173667-RtJpwOs3+0O+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2008-12-17 19:14                     ` David Teigland
2008-12-17 20:01                       ` J. Bruce Fields
2008-12-17 21:28                         ` David Teigland
2009-01-20 23:05                           ` J. Bruce Fields
2009-01-20 23:15                             ` J. Bruce Fields
2009-01-15 16:30                         ` David Teigland [this message]
2009-01-19 22:54                           ` David Teigland

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=20090115163014.GA6602@redhat.com \
    --to=teigland@redhat.com \
    --cc=bfields@fieldses.org \
    --cc=jlayton@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.