2007-02-03 05:34:15

by J.Bruce Fields

[permalink] [raw]
Subject: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

From: Marc Eshel <[email protected]> - unquoted

Factor out the code that switches between generic and filesystem-specific lock
methods; eventually we want to call this from lock managers (lockd and nfsd)
too; currently they only call the generic methods.

This patch does that for all the setlk code.

Signed-off-by: "J. Bruce Fields" <[email protected]>
---
fs/locks.c | 68 +++++++++++++++++++++++--------------------
include/linux/lockd/bind.h | 1 +
2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 95246dc..c88139d 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1691,6 +1691,21 @@ out:
return error;
}

+/**
+ * vfs_lock_file - file byte range lock
+ * @filp: The file to apply the lock to
+ * @cmd: type of locking operation (F_SETLK, F_GETLK, etc.)
+ * @fl: The lock to be applied
+ */
+int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl)
+{
+ if (filp->f_op && filp->f_op->lock)
+ return filp->f_op->lock(filp, cmd, fl);
+ else
+ return posix_lock_file(filp, fl);
+}
+EXPORT_SYMBOL(vfs_lock_file);
+
/* Apply the lock described by l to an open file descriptor.
* This implements both the F_SETLK and F_SETLKW commands of fcntl().
*/
@@ -1753,21 +1768,17 @@ again:
if (error)
goto out;

- if (filp->f_op && filp->f_op->lock != NULL)
- error = filp->f_op->lock(filp, cmd, file_lock);
- else {
- for (;;) {
- error = posix_lock_file(filp, file_lock);
- if ((error != -EAGAIN) || (cmd == F_SETLK))
- break;
- error = wait_event_interruptible(file_lock->fl_wait,
- !file_lock->fl_next);
- if (!error)
- continue;
-
- locks_delete_block(file_lock);
+ for (;;) {
+ error = vfs_lock_file(filp, cmd, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK))
break;
- }
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ locks_delete_block(file_lock);
+ break;
}

/*
@@ -1889,21 +1900,17 @@ again:
if (error)
goto out;

- if (filp->f_op && filp->f_op->lock != NULL)
- error = filp->f_op->lock(filp, cmd, file_lock);
- else {
- for (;;) {
- error = posix_lock_file(filp, file_lock);
- if ((error != -EAGAIN) || (cmd == F_SETLK64))
- break;
- error = wait_event_interruptible(file_lock->fl_wait,
- !file_lock->fl_next);
- if (!error)
- continue;
-
- locks_delete_block(file_lock);
+ for (;;) {
+ error = vfs_lock_file(filp, cmd, file_lock);
+ if ((error != -EAGAIN) || (cmd == F_SETLK64))
break;
- }
+ error = wait_event_interruptible(file_lock->fl_wait,
+ !file_lock->fl_next);
+ if (!error)
+ continue;
+
+ locks_delete_block(file_lock);
+ break;
}

/*
@@ -1940,10 +1947,7 @@ void locks_remove_posix(struct file *filp, fl_owner_t owner)
lock.fl_ops = NULL;
lock.fl_lmops = NULL;

- if (filp->f_op && filp->f_op->lock != NULL)
- filp->f_op->lock(filp, F_SETLK, &lock);
- else
- posix_lock_file(filp, &lock);
+ vfs_lock_file(filp, F_SETLK, &lock);

if (lock.fl_ops && lock.fl_ops->fl_release_private)
lock.fl_ops->fl_release_private(&lock);
diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
index f78b24a..c88da8e 100644
--- a/include/linux/lockd/bind.h
+++ b/include/linux/lockd/bind.h
@@ -39,5 +39,6 @@ extern int lockd_up(int proto);
extern void lockd_down(void);

extern int vfs_test_lock(struct file *, struct file_lock *, struct file_lock *);
+extern int vfs_lock_file(struct file *, int cmd, struct file_lock *);

#endif /* LINUX_LOCKD_BIND_H */
--
1.5.0.rc1.g72fe


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-02-03 08:51:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Sat, Feb 03, 2007 at 12:33:59AM -0500, J. Bruce Fields wrote:
> + */
> +int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl)
> +{
> + if (filp->f_op && filp->f_op->lock)
> + return filp->f_op->lock(filp, cmd, fl);
> + else
> + return posix_lock_file(filp, fl);
> +}
> +EXPORT_SYMBOL(vfs_lock_file);

_GPL please (and same for the last patch)

> + for (;;) {
> + error = vfs_lock_file(filp, cmd, file_lock);
> + if ((error != -EAGAIN) || (cmd == F_SETLK))

No need for the inside braces here.

> + error = vfs_lock_file(filp, cmd, file_lock);
> + if ((error != -EAGAIN) || (cmd == F_SETLK64))

again.


2007-02-04 01:44:31

by Brad Boyer

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Sat, Feb 03, 2007 at 08:51:36AM +0000, Christoph Hellwig wrote:
> On Sat, Feb 03, 2007 at 12:33:59AM -0500, J. Bruce Fields wrote:
> > + */
> > +int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl)
> > +{
> > + if (filp->f_op && filp->f_op->lock)
> > + return filp->f_op->lock(filp, cmd, fl);
> > + else
> > + return posix_lock_file(filp, fl);
> > +}
> > +EXPORT_SYMBOL(vfs_lock_file);
>
> _GPL please (and same for the last patch)

Any particular reason? It seems like this is a function that
would be exactly the sort of thing to be publically exported.
I know it's not a popular opinion around here, but I think
that the GPL exports should be primarily for things that
aren't intended to be used by normal modules. It seems to
me that people pushing for everything to be marked GPL are
trying to get a backdoor enforcement of their own dislike
of proprietary kernel modules in spite of Linus' known
stance on the issue. I hope this doesn't start a flamewar,
but I do want to bring up this even if many people don't
want to hear it. I'm sure I'm not the only one with this
stance on it.

Brad Boyer
[email protected]


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-02-04 01:58:13

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Sat, Feb 03, 2007 at 08:51:36AM +0000, Christoph Hellwig wrote:
> On Sat, Feb 03, 2007 at 12:33:59AM -0500, J. Bruce Fields wrote:
> > + */
> > +int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl)
> > +{
> > + if (filp->f_op && filp->f_op->lock)
> > + return filp->f_op->lock(filp, cmd, fl);
> > + else
> > + return posix_lock_file(filp, fl);
> > +}
> > +EXPORT_SYMBOL(vfs_lock_file);
>
> _GPL please (and same for the last patch)

Oops, sorry--I think you asked for that before. OK, done.

>
> > + for (;;) {
> > + error = vfs_lock_file(filp, cmd, file_lock);
> > + if ((error != -EAGAIN) || (cmd == F_SETLK))
>
> No need for the inside braces here.
>
> > + error = vfs_lock_file(filp, cmd, file_lock);
> > + if ((error != -EAGAIN) || (cmd == F_SETLK64))
>
> again.

Got it, thanks.--b.

2007-02-04 02:27:50

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Fri, Feb 02, 2007 at 09:16:38PM -0800, Brad Boyer wrote:
> On Sat, Feb 03, 2007 at 08:51:36AM +0000, Christoph Hellwig wrote:
> > On Sat, Feb 03, 2007 at 12:33:59AM -0500, J. Bruce Fields wrote:
> > > + */
> > > +int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl)
> > > +{
> > > + if (filp->f_op && filp->f_op->lock)
> > > + return filp->f_op->lock(filp, cmd, fl);
> > > + else
> > > + return posix_lock_file(filp, fl);
> > > +}
> > > +EXPORT_SYMBOL(vfs_lock_file);
> >
> > _GPL please (and same for the last patch)
>
> Any particular reason? It seems like this is a function that
> would be exactly the sort of thing to be publically exported.
> I know it's not a popular opinion around here, but I think
> that the GPL exports should be primarily for things that
> aren't intended to be used by normal modules.

Actually, this is only used by the NLM and NFSv4 servers. I doubt
there will be other users any time soon.

If you really want to argue about the EXPORT_SYMBOL_GPL stuff you'll
want to google through the archives for previous discussions first.

--b.

2007-02-04 08:37:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Fri, Feb 02, 2007 at 09:16:38PM -0800, Brad Boyer wrote:
> Any particular reason?

In general we want everything that's internal to be _GPL, and a symbol
that's only for use in the lock manager falls under that category.


2007-02-03 21:27:50

by Brad Boyer

[permalink] [raw]
Subject: Re: [PATCH 3/14] locks: factor out generic/filesystem switch from setlock code

On Sun, Feb 04, 2007 at 08:37:32AM +0000, Christoph Hellwig wrote:
> In general we want everything that's internal to be _GPL, and a symbol
> that's only for use in the lock manager falls under that category.

OK. I guess I misunderstood the purpose of the function. Thank you
for your explanation. It looked more generic than that based on a
quick read of the patch.

Brad Boyer
[email protected]