From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve French Subject: Re: fcntl method for file_operations Date: 25 Mar 2004 14:25:58 -0600 Sender: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org Message-ID: <1080246358.3200.45.camel@stevef95.austin.ibm.com> References: <1080237894.2380.5.camel@stevef95.austin.ibm.com> <20040325193545.GC25059@parcelfarce.linux.theplanet.co.uk> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org, linux-cifs-client@lists.samba.org Return-path: To: Matthew Wilcox , Jamie Lokier In-Reply-To: <20040325193545.GC25059@parcelfarce.linux.theplanet.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org List-Id: linux-fsdevel.vger.kernel.org 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