From: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
To: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org,
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH 1/2] locks: introduce i_blockleases to close lease races
Date: Sun, 12 Jun 2011 16:54:33 -0400 [thread overview]
Message-ID: <1307912073.3564.80.camel@localhost.localdomain> (raw)
In-Reply-To: <20110612191220.GK12149-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Sun, 2011-06-12 at 15:12 -0400, J. Bruce Fields wrote:
> On Sun, Jun 12, 2011 at 03:10:04PM -0400, Mimi Zohar wrote:
> > On Sun, 2011-06-12 at 00:08 -0400, J. Bruce Fields wrote:
> > > On Fri, Jun 10, 2011 at 05:34:46PM -0400, J. Bruce Fields wrote:
> > > > On Fri, Jun 10, 2011 at 04:24:00PM -0400, Mimi Zohar wrote:
> > > > > On Thu, 2011-06-09 at 20:10 -0400, J. Bruce Fields wrote:
> > > > > > From: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > > > >
> > > > > > Since break_lease is called before i_writecount is incremented, there's
> > > > > > a window between the two where a setlease call would have no way to know
> > > > > > that an open is about to happen.
> > > > >
> > > > > So unless the break_lease() call is moved from may_open() to after
> > > > > nameidata_to_filp(), I don't see any other options.
> > > >
> > > > Actually, offhand I can't see why that wouldn't be OK.
> > > >
> > > > Though I think we still end up needing something like i_blockleases to
> > > > handle unlink, link, rename, chown, and chmod.
> > >
> > > Well, I guess there's a bizarre alternative that wouldn't require a new
> > > inode field:
> >
> > In lieu of adding a new inode field, another possible option, a bit
> > kludgy, would be extending i_flock with an additional fl_flag
> > FL_BLOCKLEASE.
> >
> > #define IS_BLOCKLEASE(fl) (fl->fl_flags & FL_BLOCKLEASE)
>
> Alas, that would mean adding and removing one of these file locks around
> every single link, unlink, rename,....
>
> --b.
You're adding a call to break_lease() for each of them. Currently
__break_lease() is only called if a lease exists. Assuming there aren't
any existing leases, couldn't break_lease() call something like
block_lease()? The free would be after the link, unlink, ...,
completed/failed.
(You wouldn't actually need to alloc/free the 'struct file_lock' each
time, just set the pointer and reset to NULL.)
Mimi
> >
> > Mimi
> >
> > > What we care about is conflicts between read leases and operations that
> > > modify the metadata of the inode or the set of names pointing to it.
> > >
> > > As far as I can tell those operations all take the i_mutex either on the
> > > inode itself or on the parents of one of its aliases.
> > >
> > > So, you could prevent break_lease/setlease races by calling setlease
> > > under *all* of those i_mutexes:
> > >
> > > - take i_mutex on the inode
> > > - take i_lock to prevent the set of aliases from changing
> > > - take i_mutex for parent of each alias
> > > - set the lease
> > > - drop the parent i_mutexes, etc.
> > >
> > > where the i_mutexes would all be taken with mutex_trylock, and we'd just
> > > fail the whole setlease if any of them failed.
> > >
> > > ???
> > >
> > > --b.
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2011-06-12 20:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-10 0:09 lease/delegation/oplock semantics J. Bruce Fields
2011-06-10 0:10 ` [PATCH 1/2] locks: introduce i_blockleases to close lease races J. Bruce Fields
[not found] ` <20110610001011.GD22215-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2011-06-10 0:11 ` [PATCH 2/2] locks: break lease on unlink J. Bruce Fields
2011-06-10 20:24 ` [PATCH 1/2] locks: introduce i_blockleases to close lease races Mimi Zohar
2011-06-10 21:34 ` J. Bruce Fields
2011-06-12 4:08 ` J. Bruce Fields
2011-06-12 19:10 ` Mimi Zohar
2011-06-12 19:12 ` J. Bruce Fields
[not found] ` <20110612191220.GK12149-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2011-06-12 20:54 ` Mimi Zohar [this message]
2011-06-13 12:19 ` J. Bruce Fields
2011-06-13 20:37 ` Mimi Zohar
2011-06-14 0:35 ` J. Bruce Fields
[not found] ` <20110613121939.GL12149-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2011-06-15 15:47 ` J. Bruce Fields
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=1307912073.3564.80.camel@localhost.localdomain \
--to=zohar-23vcf4htsmix0ybbhkvfkdbpr1lh4cv8@public.gmane.org \
--cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
--cc=eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.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 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).