Re: fcntl method for file_operations

[Steve French <smfltc@us.ibm.com>]

On Thu, 2004-03-25 at 13:35, Matthew Wilcox wrote:
> On Thu, Mar 25, 2004 at 12:04:54PM -0600, Steve French wrote:
> > > the NFS developers have submitted a patch which adds an fcntl() method
> > > to the file_operations structure.
> > 
> > This patch should be helpful to the cifs vfs for a different reason
> > (than the original NFS conflict between O_DIRECT/O_APPEND) , it will
> > allow a way to get directory change notification F_NOTIFY (and file open
> > notification, F_SETLEASE, perhaps) sent across the network (currenty
> > many fcntl calls are meaningful for local filesystems only).
> 
> I don't think we should have an fcntl() method.  I'd rather see a typed
> set of method calls for whatever set of routines are actually useful.
> Something along the lines of ethtool_ops, say ;-)
> 
> perhaps the right thing to do is just to add the following ops to
> file_operations:
> 
> 	int (*fcntl_setfl)(int fd, struct file *filp, unsigned long arg);
> 	int (*fcntl_getlease)(struct file *filp);
> 	int (*fcntl_setlease)(int fd, struct file *filp, unsigned long arg);
> 	int (*fcntl_dirnotify(int fd, struct file *filp, unsigned long arg);
> 
> comments?

Sounds ok to me, the only minor drawback is that it would mean a few
more routines to be exported by the fs layer (generic_fcntl_dirnotify,
generic_fcntl_setlease etc.) rather than a single generic_fcntl.  I
don't know if there is a case in which filesystems would ever need to
get non-standard fcntls (or ones that fs/fcntl.c does not implement
yet).  The NFS guys suggestion would allow nfs to handled new fcntls by
filtering them before fcntl.c got to them.  Although I can't imagine any
reason why someone would invent a new fcntl call that I would need the
cifs vfs to filter, but perhaps it would allow other filesystems to
simplify fs tools that port to multiple operating system by adding
filesystem dependent fcntls (sounds like a bad idea though).

But I slightly prefer Jamie's suggestion of modifying the code farther
downstream (e.g. in dnotify to call out to the affected filesystem)
which is similar to what I had suggested a few months ago.  

There is another obvious hole in the vfs layer - to allow quotas to be
passed to network filesystems and filesystems without true block
devices.   Current sys_quotactl does	
	bdev = lookup_bdev(name)
then
	get_super(bdev)

rather than looking up the superblock directly from the name (using
user_path_walk as sys_statfs and other places in the kernel do) so that
small section of quota.c needs to be modified for non-local and
deviceless filesystems.

On Thu, 2004-03-25 at 13:55, Jamie Lokier wrote:
> In other words, for dnotify, the generic dnotify code should always be
> used so that the API is consistent among filesystems, but it should
> call the filesystem to do the filesystem-specific part namely
> registering a callback function to be called when a non-local change
> is detected.

> Similarly for leases, and any other wonders that are added in future
> like mount notifications