Hi Bruce,
I was just reviewing this set of patches today... I think if the idea
is not to call fl->fl_lmops->lm_release_private(fl) when the file_lock
struct was not used, then I'd prefer to introduce an exported
__locks_free_lock() function that would do it in order not to expose
the kmem_cache implementation and allow other implementations to do
it.
But I was reading the code and thinking a little more about it and now
I think the correct behavior should be to always call
fl->fl_lmops->lm_release_private(fl) (if the pointers are not NULL)
and have that function behave appropriately if the file_lock struct
was not used.
What made me think of that was a use of fl_ops (it's not directly
fl_lmops, but still I think it would be nice to keep a similar
interface) where nfs4_fl_lock_ops.fl_release_private =
nfs4_fl_release_lock and nfs4_fl_release_lock calls
nfs4_put_lock_state which frees some lists and decrements the usage
counter... Not calling fl->fl_ops->fl_release_private(fl) in that
particular case would be clearly wrong...
So I was thinking of tracking whether the lease was assigned, probably
setting the flag in vfs_setlease(), and then changing
lease_release_private_callback() to check whether the flag was set and
only resetting the F_SETOWN and F_SETSIG information if the flag was
set...
What do you think of that idea?
I just got a quick diff which outlines what I'm thinking of, I haven't
tested it yet, I'll try to build it and run it to see if it passes the
testcase. But please let me know what you think of it.
Cheers,
Fil
------- >8 cut here -------
diff --git a/fs/locks.c b/fs/locks.c
index 814c51d..242ac84 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -429,7 +429,7 @@ static void lease_break_callback(struct file_lock *fl)
static void lease_release_private_callback(struct file_lock *fl)
{
- if (!fl->fl_file)
+ if (!fl->fl_file || !fl->fl_lease_inuse)
return;
f_delown(fl->fl_file);
@@ -1513,6 +1513,9 @@ int vfs_setlease(struct file *filp, long arg,
struct file_lock **lease)
error = __vfs_setlease(filp, arg, lease);
unlock_flocks();
+ if (!error)
+ lease->fl_lease_inuse = 1;
+
return error;
}
EXPORT_SYMBOL_GPL(vfs_setlease);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 17fd887..2c577a9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1176,6 +1176,7 @@ struct file_lock {
struct list_head fl_block; /* circular list of blocked processes */
fl_owner_t fl_owner;
unsigned int fl_flags;
+ unsigned char fl_lease_inuse;
unsigned char fl_type;
unsigned int fl_pid;
struct pid *fl_nspid;
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
