NFS lockd patch proposal for user-level control of the grace period

NFS lockd patch proposal for user-level control of the grace period

[Juan Gomez]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]





Following up with Alan request here I am sending a patch for fs/lockd/svc.c
that contains the grace period control feature through the proc filesystem,
feedback & sugestions are very welcome as is the prompt inclusion in the
linux distribution :-)


(See attached file: userl-gracep-control.patch)

Regards, Juan

[-- Attachment #2: userl-gracep-control.patch --]
[-- Type: application/octet-stream, Size: 4177 bytes --]

*** svc.c.old	Tue Aug 27 20:54:01 2002
--- svc.c	Tue Aug 27 20:51:20 2002
***************
*** 35,40 ****
--- 35,42 ----
  #include <linux/lockd/lockd.h>
  #include <linux/nfs.h>
  
+ #include <linux/proc_fs.h>
+ 
  #define NLMDBG_FACILITY		NLMDBG_SVC
  #define LOCKD_BUFSIZE		(1024 + NLMSSVC_XDRSIZE)
  #define ALLOWED_SIGS		(sigmask(SIGKILL))
***************
*** 77,82 ****
--- 79,241 ----
  	nlmsvc_grace_period = 0;
  }
  
+ 
+ 
+ /*
+  * This code enable user-level control of the grace period state of the lockd
+  * daemon.
+  */
+ 
+ 
+ /* 1: nlm_proc_fs not intialized, 0: initialized */
+ static int nlm_proc_fs_init_flag = 1;
+ 
+ static unsigned long grace_period_expire;
+ 
+ 
+ 
+ 
+ /* Function used to read grace period status of lockd */
+ 
+ static ssize_t
+ nlmsvc_grace_period_read(char *page, 
+                          char **start, 
+                          off_t off,
+                          int count, 
+                          int *eof, 
+                          void *data)
+ {
+     char        *s = nlmsvc_grace_period ? "1" : "0";
+     int         len = strlen(s);
+ 
+     *eof = 0;
+     *start = page;
+ 
+     if (off < 0)
+         return -EINVAL;
+ 
+     if (off >= len){
+       *eof = 1;
+       return 0;
+     }
+ 
+     if (len < (off + count)) {
+ 
+       *eof = 1;
+       count = len - off;
+ 
+     } else {
+ 
+       count = len;
+ 
+     }
+ 
+     memcpy(page, s + off, count);
+ 
+     return count;
+ 
+ }/* static ssize_t nlmsvc_grace_period_read() */
+ 
+ 
+ /*
+  * Set status of entry/binfmt_misc:
+  * '1' enables, '0' disables and '-1' clears entry/binfmt_misc
+  */
+ static int parse_command(const char *buffer, size_t count)
+ {
+ 	char s[4];
+ 
+ 	if (!count)
+ 		return 0;
+ 
+ 	if (count > 3)
+ 		return -EINVAL;
+ 
+ 	memcpy(s, buffer, count);
+ 
+ 	if (s[count-1] == '\n')
+ 		count--;
+ 
+ 	if (count == 1 && s[0] == '0')
+ 		return 1;
+ 
+ 	if (count == 1 && s[0] == '1')
+ 		return 2;
+ 
+ 	return -EINVAL;
+ 
+ }
+ 
+ /* Function used to set/reset grace period status of lockd */
+ 
+ static ssize_t nlmsvc_grace_period_write(struct file *file, 
+                                          const char *buffer,
+                                          unsigned long count,
+                                          void *data)
+ {
+     int res = parse_command(buffer, count);
+     struct dentry *root;
+     
+     switch (res) {
+     case 1: 
+         clear_grace_period();
+         break;
+     case 2: 
+         grace_period_expire = set_grace_period();
+         break;
+     default: 
+         return res;
+     }
+     return count;
+ 
+ }/* static ssize_t nlmsvc_grace_period_write() */
+ 
+ 
+ 
+ 
+ 
+ static struct proc_dir_entry *base;
+ 
+ 
+ static void nlm_proc_fs_init()
+ {
+   struct proc_dir_entry *p;
+ 
+   if (nlm_proc_fs_init_flag){
+     
+     if (!(base = proc_mkdir("nlm", proc_root_fs)))
+       return;
+     
+     nlm_proc_fs_init_flag = 0;
+ 
+     if ((p = create_proc_entry("nlmsvc_grace_period", 0, base))) {
+ 
+       p->read_proc = nlmsvc_grace_period_read;
+       p->write_proc = nlmsvc_grace_period_write;
+ 
+     }
+     
+   }
+ 
+ }/* static int nlm_proc_fs_init() */
+ 
+ 
+ 
+ 
+ static void nlm_proc_fs_exit()
+ {
+ 
+     if (!nlm_proc_fs_init_flag && base){
+         
+       remove_proc_entry("nlmsvc_grace_period", base);
+       remove_proc_entry("nlm", proc_root_fs);
+ 
+     }
+ 
+ }/* static void nlm_proc_fs_exit() */
+ 
+ 
+ 
  /*
   * This is the lockd kernel thread
   */
***************
*** 85,91 ****
  {
  	struct svc_serv	*serv = rqstp->rq_server;
  	int		err = 0;
- 	unsigned long grace_period_expire;
  
  	/* Lock module and set up kernel thread */
  	MOD_INC_USE_COUNT;
--- 244,249 ----
***************
*** 228,233 ****
--- 386,393 ----
  	if (nlmsvc_pid)
  		goto out;
  
+         nlm_proc_fs_init();
+ 
  	/*
  	 * Sanity check: if there's no pid,
  	 * we should be the first user ...
***************
*** 291,296 ****
--- 451,459 ----
  			goto out;
  	} else
  		printk(KERN_WARNING "lockd_down: no users! pid=%d\n", nlmsvc_pid);
+ 
+         nlm_proc_fs_exit();
+ 
  
  	if (!nlmsvc_pid) {
  		if (warned++ == 0)

Re: NFS lockd patch proposal for user-level control of the grace period

[Christoph Hellwig]

On Tue, Aug 27, 2002 at 03:06:33PM -0600, Juan Gomez wrote:
> Following up with Alan request here I am sending a patch for fs/lockd/svc.c
> that contains the grace period control feature through the proc filesystem,
> feedback & sugestions are very welcome as is the prompt inclusion in the
> linux distribution :-)

What do you need this for eaxctly?

> (See attached file: userl-gracep-control.patch)

Would be much nicer if inlined and in unified diff format.

Please follow Documentation/CodingStyle and i think for that purpose sysctl
are much better than procfs

Re: NFS lockd patch proposal for user-level control of the grace period

[Juan Gomez]

I think either way we get to the kernel would be ok with me. I picked
sysctl because some comments in the code susggested somebody already though
of supporting a sysctl gate to user land to export
grace period control, also Chrisptop suggested this was the way to go when
I previously proposed using the raw proc filesystem. I think what is nice
about sysctl is that it is extensible, If we use signals and some other
folks keep on using signals for other control purposed then they will
eventually run out and they will have to add sysctl anyway. Said that I
think either way serves the purpose I want.


Juan



|---------+---------------------------------->
|         |           Neil Brown             |
|         |           <neilb@cse.unsw.edu.au>|
|         |           Sent by:               |
|         |           linux-kernel-owner@vger|
|         |           .kernel.org            |
|         |                                  |
|         |                                  |
|         |           09/08/02 05:27 PM      |
|         |                                  |
|---------+---------------------------------->
  >-----------------------------------------------------------------------------------------------------------------------------|
  |                                                                                                                             |
  |       To:       Juan Gomez/Almaden/IBM@IBMUS                                                                                |
  |       cc:       Alan Cox <alan@lxorguk.ukuu.org.uk>, hch@infradead.org, linux-kernel@vger.kernel.org                        |
  |       Subject:  Re: NFS lockd patch proposal for user-level control of the grace period                                     |
  |                                                                                                                             |
  |                                                                                                                             |
  >-----------------------------------------------------------------------------------------------------------------------------|



On Friday September 6, juang@us.ibm.com wrote:
>
>
>
>
> Christoph, Alan, Neil,
>
> Attached you will find the patch with the sysctl implementation of my
> previous patch to enable grace period control from user-land.
> Please let me know if this looks good enough for inclusion in the kernel
> distribution or whether I still need to do something else.
> Note this piece is derived from net/sunrpc/sysctl.c, which by the way I
> think has a problem with the READ/WRITE verifys which seem
>  to be swicthed which I fixed in lockd version but not there, you may
want
> to take a look at net/sunrpc/sysctl.c and fix that although that's a
minor
> thing.
>
> (See attached file: lockd-sysctl.patch)
>

I still haven't managed to find out exactly what you want to do with
this, and hence whether it is appropriate.

You mentioned in another Email that this was for a High Availability
setup where one server might take-over a filesystem that another
server was previously serving.

If this is the case, do you really want to change the grace period, or
do you really want to re-start the grace period.
If that is what you really want, then I think that sysctl is
un-necessary and a simple signal would do the trick.
Currently, SIGKILL will
   1/ drop all locks held for clients
   2/ restart the grace period.

it would probably be quite sensible (and trivial to code) for SIGHUP
(say) to restart the grace period without dropping the locks.

Would this be suitable for you?

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: NFS lockd patch proposal for user-level control of the grace period

[Neil Brown]

On Friday September 6, juang@us.ibm.com wrote:
> 
> 
> 
> 
> Christoph, Alan, Neil,
> 
> Attached you will find the patch with the sysctl implementation of my
> previous patch to enable grace period control from user-land.
> Please let me know if this looks good enough for inclusion in the kernel
> distribution or whether I still need to do something else.
> Note this piece is derived from net/sunrpc/sysctl.c, which by the way I
> think has a problem with the READ/WRITE verifys which seem
>  to be swicthed which I fixed in lockd version but not there, you may want
> to take a look at net/sunrpc/sysctl.c and fix that although that's a minor
> thing.
> 
> (See attached file: lockd-sysctl.patch)
> 

I still haven't managed to find out exactly what you want to do with
this, and hence whether it is appropriate.

You mentioned in another Email that this was for a High Availability
setup where one server might take-over a filesystem that another
server was previously serving.

If this is the case, do you really want to change the grace period, or
do you really want to re-start the grace period.
If that is what you really want, then I think that sysctl is
un-necessary and a simple signal would do the trick.
Currently, SIGKILL will
   1/ drop all locks held for clients
   2/ restart the grace period.

it would probably be quite sensible (and trivial to code) for SIGHUP
(say) to restart the grace period without dropping the locks.

Would this be suitable for you?

NeilBrown

Re: NFS lockd patch proposal for user-level control of the grace period

[Daniel Egger]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

Am Sam, 2002-09-07 um 03.36 schrieb Christoph Hellwig:

> Any chance you could resend this as a unified diff and with a proper mailer
> so I actually have a chance to read it?

It's perfectly readable here except for not being in unified form which
is a minor problem.... maybe you should go check your own mailer....
 
-- 
Servus,
       Daniel

[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: NFS lockd patch proposal for user-level control of the grace period

[Christoph Hellwig]

On Fri, Sep 06, 2002 at 04:36:39PM -0600, Juan Gomez wrote:
> Christoph, Alan, Neil,

Any chance you could resend this as a unified diff and with a proper mailer
so I actually have a chance to read it?

Thanks in advance,

	Christoph

Re: NFS lockd patch proposal for user-level control of the grace period

[Juan Gomez]

[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]





Christoph, Alan, Neil,

Attached you will find the patch with the sysctl implementation of my
previous patch to enable grace period control from user-land.
Please let me know if this looks good enough for inclusion in the kernel
distribution or whether I still need to do something else.
Note this piece is derived from net/sunrpc/sysctl.c, which by the way I
think has a problem with the READ/WRITE verifys which seem
 to be swicthed which I fixed in lockd version but not there, you may want
to take a look at net/sunrpc/sysctl.c and fix that although that's a minor
thing.

(See attached file: lockd-sysctl.patch)


Regards, Juan



|---------+---------------------------->
|         |           Alan Cox         |
|         |           <alan@lxorguk.   |
|         |           ukuu.org.uk>     |
|         |                            |
|         |           09/05/02 11:37 AM|
|         |                            |
|---------+---------------------------->
  >-----------------------------------------------------------------------------------------------------------------------------|
  |                                                                                                                             |
  |       To:       Juan Gomez/Almaden/IBM@IBMUS                                                                                |
  |       cc:                                                                                                                   |
  |       Subject:  Re: NFS lockd patch proposal for user-level control of the grace  period                                    |
  |                                                                                                                             |
  |                                                                                                                             |
  >-----------------------------------------------------------------------------------------------------------------------------|



I was waiting for a version that used the sysctl /proc/sys interface
instead. The concept of the interface is clearly fine



[-- Attachment #2: lockd-sysctl.patch --]
[-- Type: application/octet-stream, Size: 7598 bytes --]

diff -rcN linux-2.4.19/fs/lockd/Makefile linux-2.4.19-lockd-sysctl/fs/lockd/Makefile
*** linux-2.4.19/fs/lockd/Makefile	Fri Dec 29 14:07:23 2000
--- linux-2.4.19-lockd-sysctl/fs/lockd/Makefile	Fri Sep  6 19:09:02 2002
***************
*** 12,18 ****
  export-objs := lockd_syms.o
  
  obj-y    := clntlock.o clntproc.o host.o svc.o svclock.o svcshare.o \
! 	    svcproc.o svcsubs.o mon.o xdr.o lockd_syms.o
  
  obj-$(CONFIG_LOCKD_V4) += xdr4.o svc4proc.o
  
--- 12,18 ----
  export-objs := lockd_syms.o
  
  obj-y    := clntlock.o clntproc.o host.o svc.o svclock.o svcshare.o \
! 	    svcproc.o svcsubs.o mon.o xdr.o lockd_syms.o sysctl.o
  
  obj-$(CONFIG_LOCKD_V4) += xdr4.o svc4proc.o
  
diff -rcN linux-2.4.19/fs/lockd/svc.c linux-2.4.19-lockd-sysctl/fs/lockd/svc.c
*** linux-2.4.19/fs/lockd/svc.c	Sun Oct 21 10:32:33 2001
--- linux-2.4.19-lockd-sysctl/fs/lockd/svc.c	Fri Sep  6 21:44:32 2002
***************
*** 58,63 ****
--- 58,69 ----
  unsigned long			nlm_timeout = LOCKD_DFLT_TIMEO;
  unsigned long			nlm_udpport, nlm_tcpport;
  
+ /* Imports needed to support sysctl */
+ extern void             lockd_register_sysctl(void);
+ extern void             lockd_deregister_sysctl(void);
+ 
+ static unsigned long            grace_period_expire;
+ 
  static unsigned long set_grace_period(void)
  {
  	unsigned long grace_period;
***************
*** 72,82 ****
  	return grace_period + jiffies;
  }
  
! static inline void clear_grace_period(void)
  {
  	nlmsvc_grace_period = 0;
  }
  
  /*
   * This is the lockd kernel thread
   */
--- 78,97 ----
  	return grace_period + jiffies;
  }
  
! /* Public version of set_grace_period used from sysctl.c */
! inline void start_grace_period(void)
! {
! 
! 	grace_period_expire = set_grace_period();
! 
! }
! 
! inline void clear_grace_period(void)
  {
  	nlmsvc_grace_period = 0;
  }
  
+ 
  /*
   * This is the lockd kernel thread
   */
***************
*** 85,91 ****
  {
  	struct svc_serv	*serv = rqstp->rq_server;
  	int		err = 0;
- 	unsigned long grace_period_expire;
  
  	/* Lock module and set up kernel thread */
  	MOD_INC_USE_COUNT;
--- 100,105 ----
***************
*** 228,233 ****
--- 242,249 ----
  	if (nlmsvc_pid)
  		goto out;
  
+ 	lockd_register_sysctl();
+ 
  	/*
  	 * Sanity check: if there's no pid,
  	 * we should be the first user ...
***************
*** 291,296 ****
--- 307,314 ----
  			goto out;
  	} else
  		printk(KERN_WARNING "lockd_down: no users! pid=%d\n", nlmsvc_pid);
+ 
+ 	lockd_deregister_sysctl();
  
  	if (!nlmsvc_pid) {
  		if (warned++ == 0)
diff -rcN linux-2.4.19/fs/lockd/sysctl.c linux-2.4.19-lockd-sysctl/fs/lockd/sysctl.c
*** linux-2.4.19/fs/lockd/sysctl.c	Wed Dec 31 16:00:00 1969
--- linux-2.4.19-lockd-sysctl/fs/lockd/sysctl.c	Fri Sep  6 19:08:18 2002
***************
*** 0 ****
--- 1,157 ----
+ 
+ #include <linux/types.h>
+ #include <linux/ctype.h>
+ #include <linux/sysctl.h>
+ #include <asm/errno.h>
+ #include <asm/uaccess.h>
+ #include <linux/kernel.h>
+ #include <linux/file.h>
+ 
+ static struct ctl_table_header  *lockd_table_header;
+ static ctl_table		lockd_table[];
+ 
+ /* Stuff imported from svc.c */
+ 
+ extern inline void start_grace_period(void);
+ extern inline void clear_grace_period(void);
+ extern int nlmsvc_grace_period;
+ 
+ 
+ /* Register with sysctl so we can export control of lockd to user-land
+  * via /proc/sys
+  */
+ 
+ void lockd_register_sysctl()
+ {
+ 
+   if (!lockd_table_header) {
+ 
+     printk("lockd_register_sysctl:=>register_sysctl_table()\n");
+     lockd_table_header = register_sysctl_table(lockd_table, 1);
+ 
+   }
+ 	
+ 
+ }/* void lockd_register_sysctl() */
+ 
+ 
+ 
+ 
+ /* De-register with sysctl so we do not have stale entries in 
+  * /proc/sys
+  */
+ 
+ void lockd_deregister_sysctl()
+ {
+ 
+ 	if (lockd_table_header) {
+ 	        printk("lockd_deregister_sysctl:=>register_sysctl_table()\n");
+ 		unregister_sysctl_table(lockd_table_header);
+ 		lockd_table_header = NULL;
+ 	}
+ 
+ }/* void lockd_deregister_sysctl() */
+ 
+ static int
+ proc_do_lockd_grace_period(ctl_table *table, 
+                            int write, 
+                            struct file *file,
+                            void *buffer, 
+                            size_t *lenp)
+ {
+ 	char		tmpbuf[20], *p, c;
+ 	unsigned int	value;
+ 	size_t		left, len;
+ 
+ 	printk("proc_do_lockd_grace_period: write=%d, lenp=%p, buffer=%p\n",
+ 	       write, lenp, buffer);
+ 	printk("proc_do_lockd_grace_period: len=%d\n", lenp ? *lenp : 0);
+ 
+ 	if ((file->f_pos && !write) || !*lenp) {
+ 		*lenp = 0;
+ 		return 0;
+ 	}
+ 
+ 	left = *lenp;
+ 
+ 	if (write) {
+ 
+           if (!access_ok(VERIFY_WRITE, buffer, left))
+             return -EFAULT;
+           p = (char *) buffer;
+           while (left && __get_user(c, p) >= 0 && isspace(c))
+             left--, p++;
+           if (!left)
+             goto done;
+           
+           if (left > sizeof(tmpbuf) - 1)
+             return -EINVAL;
+           copy_from_user(tmpbuf, p, left);
+           tmpbuf[left] = '\0';
+ 
+ 
+ 	  for (p = tmpbuf, value = 0; '0' <= *p && *p <= '9'; p++, left--)
+ 	    value = 10 * value + (*p - '0');
+ 	  if (*p && !isspace(*p))
+ 	    return -EINVAL;
+ 	  while (left && isspace(*p))
+ 	    left--, p++;
+ 
+           if (value == 0) {
+ 
+             clear_grace_period();
+ 
+           } else if(value == 1) {
+ 
+             start_grace_period();
+ 
+           } else {
+ 
+             return -EINVAL;
+ 
+           }
+           
+ 
+ 	} else {
+           /* Read Access */
+ 
+ 	  printk("proc_do_lockd_grace_period: reading...\n");
+ 
+           if (!access_ok(VERIFY_READ, buffer, left))
+             return -EFAULT;
+ 
+           len = sprintf(tmpbuf, "%d", nlmsvc_grace_period);
+           if (len > left)
+             len = left;
+           copy_to_user(buffer, tmpbuf, len);
+           if ((left -= len) > 0) {
+             put_user('\n', (char *)buffer + len);
+             left--;
+           }
+ 	}
+ 
+ done:
+ 	*lenp -= left;
+ 	file->f_pos += *lenp;
+ 	return 0;
+ 
+ }/*static int proc_do_lockd_sysctl() */
+ 
+ /* Define dir structure we want to export through /proc/fs */
+ 
+ #define DIRENTRY(nam1, nam2, child)	\
+ 	{CTL_##nam1, #nam2, NULL, 0, 0555, child }
+ #define LOCKD_FILE_ENTRY(nam1, nam2)	\
+ 	{LOCKD_CTL_##nam1##, "lockd_" #nam2, NULL, 0,\
+ 	 0644, NULL, &proc_do_lockd_grace_period}
+ 
+ static ctl_table		lockd_file_table[] = {
+ 	LOCKD_FILE_ENTRY(GRACE_PERIOD,  grace_period),
+ 	{0}
+ };
+ 
+ static ctl_table		lockd_table[] = {
+ 	DIRENTRY(LOCKD, lockd, lockd_file_table),
+ 	{0}
+ };
+ 
diff -rcN linux-2.4.19/include/linux/sysctl.h linux-2.4.19-lockd-sysctl/include/linux/sysctl.h
*** linux-2.4.19/include/linux/sysctl.h	Fri Aug  2 17:39:46 2002
--- linux-2.4.19-lockd-sysctl/include/linux/sysctl.h	Fri Sep  6 21:47:43 2002
***************
*** 63,69 ****
  	CTL_DEV=7,		/* Devices */
  	CTL_BUS=8,		/* Busses */
  	CTL_ABI=9,		/* Binary emulation */
! 	CTL_CPU=10		/* CPU stuff (speed scaling, etc) */
  };
  
  /* CTL_BUS names: */
--- 63,70 ----
  	CTL_DEV=7,		/* Devices */
  	CTL_BUS=8,		/* Busses */
  	CTL_ABI=9,		/* Binary emulation */
! 	CTL_CPU=10,		/* CPU stuff (speed scaling, etc) */
!         CTL_LOCKD=11,           /* Lockd info and control */
  };
  
  /* CTL_BUS names: */
***************
*** 627,632 ****
--- 628,641 ----
  	ABI_TRACE=5,		/* tracing flags */
  	ABI_FAKE_UTSNAME=6,	/* fake target utsname information */
  };
+ 
+ 
+ /* /proc/sys/lockd */
+ enum
+ {
+         LOCKD_CTL_GRACE_PERIOD=1,/* Enable user-level grace period control */
+ };
+ 
  
  #ifdef __KERNEL__