On Sun, 24 Aug 2014 09:10:46 -0700
Christoph Hellwig wrote:
> One more wishlist item in addition to the one mentioned in the patches:
>
> - add a return value to lm_break so that the lock manager can tell the
>core code "you can delete this lease right now". That gets rid of
>the games
On Sun, 24 Aug 2014 09:06:34 -0700
Christoph Hellwig wrote:
> Looks good,
>
> Reviewed-by: Christoph Hellwig
>
> Some comments on further work I'd like to see in this area, though:
>
> > + spin_lock(&inode->i_lock);
> > + time_out_leases(inode);
> > for (before = &inode->i_flock;
> >
On Sun, 24 Aug 2014 09:08:04 -0700
Christoph Hellwig wrote:
> On Sat, Aug 23, 2014 at 10:41:17AM -0400, Jeff Layton wrote:
> > There was only one place where we still could free a file_lock while
> > holding the i_lock -- lease_modify. Add a new list_head argument to the
> > lm_change operation,
On Sun, 24 Aug 2014 08:58:58 -0700
Christoph Hellwig wrote:
> I like this change a lot! But one caveat:
>
> > + /*
> > +* Despite the fact that it's an int return function, __f_setown never
> > +* returns an error. Just ignore any error return here, but spew a
> > +* WARN_ON_ONCE
On Sun, Aug 24, 2014 at 09:06:34AM -0700, Christoph Hellwig wrote:
> We really should split a lm_release from lm_change, the way it is
> used is highly confusing. In addition I think a lot of code
> currently in lease_modify should move here instead, e.g. something like:
At this point the old lm_
One more wishlist item in addition to the one mentioned in the patches:
- add a return value to lm_break so that the lock manager can tell the
core code "you can delete this lease right now". That gets rid of
the games with the timeout which require all kinds of race avoidance
code in t
On Sat, Aug 23, 2014 at 10:41:17AM -0400, Jeff Layton wrote:
> There was only one place where we still could free a file_lock while
> holding the i_lock -- lease_modify. Add a new list_head argument to the
> lm_change operation, pass in a private list when calling it, and fix
> those callers to dis
Looks good,
Reviewed-by: Christoph Hellwig
Some comments on further work I'd like to see in this area, though:
> + spin_lock(&inode->i_lock);
> + time_out_leases(inode);
> for (before = &inode->i_flock;
> ((fl = *before) != NULL) && IS_LEASE(fl);
>
I like this change a lot! But one caveat:
> + /*
> + * Despite the fact that it's an int return function, __f_setown never
> + * returns an error. Just ignore any error return here, but spew a
> + * WARN_ON_ONCE in case this ever changes.
> + */
> + WARN_ON_ONCE(__f_se
On Sun, Aug 24, 2014 at 06:08:01AM -0400, Jeff Layton wrote:
> > Can you just return -EEXIST if reusing an existing one and make it a
> > normal private pointer a we use elsewhere?
> >
>
> That sounds a little confusing...
>
> We have two pointers we pass down to generic_setlease: the file_lock
On Sat, Aug 23, 2014 at 10:41:13AM -0400, Jeff Layton wrote:
> Now that we don't need to pass in an actual lease pointer to
> vfs_setlease on unlock, we can stop tracking a pointer to the lease in
> the nfs4_file.
>
> Switch all of the places that check the fi_lease to check fi_deleg_file
> instea
On Sat, Aug 23, 2014 at 10:41:12AM -0400, Jeff Layton wrote:
> Some of the latter paragraphs seem ambiguous and just plain wrong.
> In particular the break_lease comment makes no sense. We call
> break_lease (and break_deleg) from all sorts of vfs-layer functions,
> so there is clearly such a metho
On Sat, Aug 23, 2014 at 10:41:10AM -0400, Jeff Layton wrote:
> It's unlikely to ever occur, but if there were already a lease set on
> the file then we could end up getting back a different pointer on a
> successful setlease attempt than the one we allocated. If that happens,
> the one we allocated
On Sat, Aug 23, 2014 at 10:41:09AM -0400, Jeff Layton wrote:
> lease_get_mtime is called without the i_lock held, so there's no
> guarantee about the stability of the list. Between the time when we
> assign "flock" and then dereference it to check whether it's a lease
> and for write, the lease cou
On Sat, 23 Aug 2014 18:27:57 -0700
Christoph Hellwig wrote:
> On Sat, Aug 23, 2014 at 10:41:11AM -0400, Jeff Layton wrote:
> > +static int generic_delete_lease(struct file *filp)
> > {
> > struct file_lock *fl, **before;
> > struct dentry *dentry = filp->f_path.dentry;
> > struct ino
On Sat, 23 Aug 2014 18:33:05 -0700
Christoph Hellwig wrote:
> On Sat, Aug 23, 2014 at 10:41:14AM -0400, Jeff Layton wrote:
> > In later patches, we're going to add a new lock_manager_operation to
> > finish setting up the lease while still holding the i_lock. To do
> > this, we'll need to pass a
16 matches
Mail list logo
