Re: [PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-30 Thread Christoph Hellwig
On Sun, Oct 28, 2007 at 08:40:56PM -0400, Erez Zadok wrote:
 +/**
 + * vfs_ioctl - call filesystem specific ioctl methods
 + *
 + * @filp: [in] open file to invoke ioctl method on
 + * @cmd:  [in] ioctl command to execute
 + * @arg:  [in/out] command-specific argument for ioctl

I've never seen these in/out annotations and doubt they're valid in
kerneldoc.  Randy?

 + * Invokes filesystem specific -unlock_ioctl, if one exists; otherwise

unlocked_ioctl

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-30 Thread Randy Dunlap
On Tue, 30 Oct 2007 09:56:53 + Christoph Hellwig wrote:

 On Sun, Oct 28, 2007 at 08:40:56PM -0400, Erez Zadok wrote:
  +/**
  + * vfs_ioctl - call filesystem specific ioctl methods
  + *
  + * @filp: [in] open file to invoke ioctl method on
  + * @cmd:  [in] ioctl command to execute
  + * @arg:  [in/out] command-specific argument for ioctl
 
 I've never seen these in/out annotations and doubt they're valid in
 kerneldoc.  Randy?

They are just treated as part of the parameter explanation text.
I don't see any problem with them.

  + * Invokes filesystem specific -unlock_ioctl, if one exists; otherwise
 
 unlocked_ioctl


---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-30 Thread Randy Dunlap
On Tue, 30 Oct 2007 17:14:36 + Christoph Hellwig wrote:

 On Tue, Oct 30, 2007 at 08:22:40AM -0700, Randy Dunlap wrote:
  They are just treated as part of the parameter explanation text.
  I don't see any problem with them.
 
 Well, it's completely inconsistant with any other kerneldoc..

True.  and it has an advantage.

---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-30 Thread Erez Zadok
In message [EMAIL PROTECTED], Christoph Hellwig writes:
 On Tue, Oct 30, 2007 at 08:22:40AM -0700, Randy Dunlap wrote:
  They are just treated as part of the parameter explanation text.
  I don't see any problem with them.
 
 Well, it's completely inconsistant with any other kerneldoc..

If it doesn't hurt, and kerneldoc will present it as such, then I'd leave
the [in/out] in place.  Ioctls are the few places where you can have
variables used as both input and output; so _somehow_ we need to capture
that behavior.

Erez.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-30 Thread Erez Zadok
Rename old vfs_ioctl to do_ioctl, because the comment above it clearly
indicates that it is an internal function not to be exported to modules;
therefore it should have a more traditional do_XXX name.  The new do_ioctl
is exported in fs.h but not to modules.

Rename the old do_ioctl to vfs_ioctl because the names vfs_XXX should
preferably be reserved to callable VFS functions which modules may call, as
many other vfs_XXX functions already do.  Export the new vfs_ioctl to GPL
modules so others can use it (including Unionfs and eCryptfs).  Add DocBook
for new vfs_ioctl.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/compat_ioctl.c  |2 +-
 fs/ioctl.c |   29 +
 include/linux/fs.h |4 +++-
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index a4284cc..a1604ce 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -2972,7 +2972,7 @@ asmlinkage long compat_sys_ioctl(unsigned int fd, 
unsigned int cmd,
}
 
  do_ioctl:
-   error = vfs_ioctl(filp, fd, cmd, arg);
+   error = do_ioctl(filp, fd, cmd, arg);
  out_fput:
fput_light(filp, fput_needed);
  out:
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 652cacf..1ab7b7d 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -16,8 +16,20 @@
 
 #include asm/ioctls.h
 
-static long do_ioctl(struct file *filp, unsigned int cmd,
-   unsigned long arg)
+/**
+ * vfs_ioctl - call filesystem specific ioctl methods
+ * @filp: [in] open file to invoke ioctl method on
+ * @cmd:  [in] ioctl command to execute
+ * @arg:  [in/out] command-specific argument for ioctl
+ *
+ * Invokes filesystem specific -unlocked_ioctl, if one exists; otherwise
+ * invokes * filesystem specific -ioctl method.  If neither method exists,
+ * returns -ENOTTY.
+ *
+ * Returns 0 on success, -errno on error.
+ */
+long vfs_ioctl(struct file *filp, unsigned int cmd,
+  unsigned long arg)
 {
int error = -ENOTTY;
 
@@ -39,6 +51,7 @@ static long do_ioctl(struct file *filp, unsigned int cmd,
  out:
return error;
 }
+EXPORT_SYMBOL_GPL(vfs_ioctl);
 
 static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
@@ -72,18 +85,18 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
return put_user(i_size_read(inode) - filp-f_pos, p);
}
 
-   return do_ioctl(filp, cmd, arg);
+   return vfs_ioctl(filp, cmd, arg);
 }
 
 /*
  * When you add any new common ioctls to the switches above and below
  * please update compat_sys_ioctl() too.
  *
- * vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
+ * do_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
  * It's just a simple helper for sys_ioctl and compat_sys_ioctl.
  */
-int vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
- unsigned long arg)
+int do_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
+unsigned long arg)
 {
unsigned int flag;
int on, error = 0;
@@ -152,7 +165,7 @@ int vfs_ioctl(struct file *filp, unsigned int fd, unsigned 
int cmd,
if (S_ISREG(filp-f_path.dentry-d_inode-i_mode))
error = file_ioctl(filp, cmd, arg);
else
-   error = do_ioctl(filp, cmd, arg);
+   error = vfs_ioctl(filp, cmd, arg);
break;
}
return error;
@@ -172,7 +185,7 @@ asmlinkage long sys_ioctl(unsigned int fd, unsigned int 
cmd, unsigned long arg)
if (error)
goto out_fput;
 
-   error = vfs_ioctl(filp, fd, cmd, arg);
+   error = do_ioctl(filp, fd, cmd, arg);
  out_fput:
fput_light(filp, fput_needed);
  out:
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b3ec4a4..d513f16 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1924,7 +1924,9 @@ extern int vfs_stat_fd(int dfd, char __user *, struct 
kstat *);
 extern int vfs_lstat_fd(int dfd, char __user *, struct kstat *);
 extern int vfs_fstat(unsigned int, struct kstat *);
 
-extern int vfs_ioctl(struct file *, unsigned int, unsigned int, unsigned long);
+extern long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
+extern int do_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
+   unsigned long arg);
 
 extern void get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
-- 
1.5.2.2

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-28 Thread Erez Zadok
Rename old vfs_ioctl to do_ioctl, because the comment above it clearly
indicates that it is an internal function not to be exported to modules;
therefore it should have a more traditional do_XXX name.  The new do_ioctl
is exported in fs.h but not to modules.

Rename the old do_ioctl to vfs_ioctl because the names vfs_XXX should
preferably be reserved to callable VFS functions which modules may call, as
many other vfs_XXX functions already do.  Export the new vfs_ioctl to GPL
modules so others can use it (including Unionfs and eCryptfs).  Add DocBook
for new vfs_ioctl.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/compat_ioctl.c  |2 +-
 fs/ioctl.c |   30 ++
 include/linux/fs.h |3 ++-
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index a4284cc..a1604ce 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -2972,7 +2972,7 @@ asmlinkage long compat_sys_ioctl(unsigned int fd, 
unsigned int cmd,
}
 
  do_ioctl:
-   error = vfs_ioctl(filp, fd, cmd, arg);
+   error = do_ioctl(filp, fd, cmd, arg);
  out_fput:
fput_light(filp, fput_needed);
  out:
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 652cacf..34e3f58 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -16,8 +16,21 @@
 
 #include asm/ioctls.h
 
-static long do_ioctl(struct file *filp, unsigned int cmd,
-   unsigned long arg)
+/**
+ * vfs_ioctl - call filesystem specific ioctl methods
+ *
+ * @filp: [in] open file to invoke ioctl method on
+ * @cmd:  [in] ioctl command to execute
+ * @arg:  [in/out] command-specific argument for ioctl
+ *
+ * Invokes filesystem specific -unlock_ioctl, if one exists; otherwise
+ * invokes * filesystem specific -ioctl method.  If neither method exists,
+ * returns -ENOTTY.
+ *
+ * Returns 0 on success, -errno on error.
+ */
+long vfs_ioctl(struct file *filp, unsigned int cmd,
+  unsigned long arg)
 {
int error = -ENOTTY;
 
@@ -39,6 +52,7 @@ static long do_ioctl(struct file *filp, unsigned int cmd,
  out:
return error;
 }
+EXPORT_SYMBOL_GPL(vfs_ioctl);
 
 static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
@@ -72,18 +86,18 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
return put_user(i_size_read(inode) - filp-f_pos, p);
}
 
-   return do_ioctl(filp, cmd, arg);
+   return vfs_ioctl(filp, cmd, arg);
 }
 
 /*
  * When you add any new common ioctls to the switches above and below
  * please update compat_sys_ioctl() too.
  *
- * vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
+ * do_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
  * It's just a simple helper for sys_ioctl and compat_sys_ioctl.
  */
-int vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
- unsigned long arg)
+int do_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
+unsigned long arg)
 {
unsigned int flag;
int on, error = 0;
@@ -152,7 +166,7 @@ int vfs_ioctl(struct file *filp, unsigned int fd, unsigned 
int cmd,
if (S_ISREG(filp-f_path.dentry-d_inode-i_mode))
error = file_ioctl(filp, cmd, arg);
else
-   error = do_ioctl(filp, cmd, arg);
+   error = vfs_ioctl(filp, cmd, arg);
break;
}
return error;
@@ -172,7 +186,7 @@ asmlinkage long sys_ioctl(unsigned int fd, unsigned int 
cmd, unsigned long arg)
if (error)
goto out_fput;
 
-   error = vfs_ioctl(filp, fd, cmd, arg);
+   error = do_ioctl(filp, fd, cmd, arg);
  out_fput:
fput_light(filp, fput_needed);
  out:
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b3ec4a4..c0c5d36 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1924,7 +1924,8 @@ extern int vfs_stat_fd(int dfd, char __user *, struct 
kstat *);
 extern int vfs_lstat_fd(int dfd, char __user *, struct kstat *);
 extern int vfs_fstat(unsigned int, struct kstat *);
 
-extern int vfs_ioctl(struct file *, unsigned int, unsigned int, unsigned long);
+extern long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
+extern int do_ioctl(struct file *, unsigned int, unsigned int, unsigned long);
 
 extern void get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
-- 
1.5.2.2

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] VFS: swap do_ioctl and vfs_ioctl names

2007-10-28 Thread Randy Dunlap
On Sun, 28 Oct 2007 20:40:56 -0400 Erez Zadok wrote:

 Rename old vfs_ioctl to do_ioctl, because the comment above it clearly
 indicates that it is an internal function not to be exported to modules;
 therefore it should have a more traditional do_XXX name.  The new do_ioctl
 is exported in fs.h but not to modules.
 
 Rename the old do_ioctl to vfs_ioctl because the names vfs_XXX should
 preferably be reserved to callable VFS functions which modules may call, as
 many other vfs_XXX functions already do.  Export the new vfs_ioctl to GPL
 modules so others can use it (including Unionfs and eCryptfs).  Add DocBook
 for new vfs_ioctl.
 
 Signed-off-by: Erez Zadok [EMAIL PROTECTED]
 ---
  fs/compat_ioctl.c  |2 +-
  fs/ioctl.c |   30 ++
  include/linux/fs.h |3 ++-
  3 files changed, 25 insertions(+), 10 deletions(-)
 

 diff --git a/fs/ioctl.c b/fs/ioctl.c
 index 652cacf..34e3f58 100644
 --- a/fs/ioctl.c
 +++ b/fs/ioctl.c
 @@ -16,8 +16,21 @@
  
  #include asm/ioctls.h
  
 -static long do_ioctl(struct file *filp, unsigned int cmd,
 - unsigned long arg)
 +/**
 + * vfs_ioctl - call filesystem specific ioctl methods
 + *

No blank line allowed in kernel-doc between function name and its
parameters.

 + * @filp: [in] open file to invoke ioctl method on
 + * @cmd:  [in] ioctl command to execute
 + * @arg:  [in/out] command-specific argument for ioctl
 + *
 + * Invokes filesystem specific -unlock_ioctl, if one exists; otherwise
 + * invokes * filesystem specific -ioctl method.  If neither method exists,
 + * returns -ENOTTY.
 + *
 + * Returns 0 on success, -errno on error.
 + */
 +long vfs_ioctl(struct file *filp, unsigned int cmd,
 +unsigned long arg)
  {
   int error = -ENOTTY;
  

 diff --git a/include/linux/fs.h b/include/linux/fs.h
 index b3ec4a4..c0c5d36 100644
 --- a/include/linux/fs.h
 +++ b/include/linux/fs.h
 @@ -1924,7 +1924,8 @@ extern int vfs_stat_fd(int dfd, char __user *, struct 
 kstat *);
  extern int vfs_lstat_fd(int dfd, char __user *, struct kstat *);
  extern int vfs_fstat(unsigned int, struct kstat *);
  
 -extern int vfs_ioctl(struct file *, unsigned int, unsigned int, unsigned 
 long);
 +extern long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long 
 arg);
 +extern int do_ioctl(struct file *, unsigned int, unsigned int, unsigned 
 long);

Use/keep parameter names, please.  That is preferred.

  extern void get_filesystem(struct file_system_type *fs);
  extern void put_filesystem(struct file_system_type *fs);
 -- 
 1.5.2.2
 
 -
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to [EMAIL PROTECTED]
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
 


---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html