[PVFS-developers] Signal interruption in kpvfsdev.c

[Rob Ross]

Murali,

I just applied this patch, but unfortunately it wasn't totally clean (I 
had to do some things myself).  All the ambiguities were in kpvfsdev.c.  
If you could have a look at it some time, that would be great.

Thanks,

Rob

On Wed, 25 Jun 2003, Murali Vilayannur wrote:

> Hi,
> 
> > used).  Applying Murali's "brutal" sig-hack.patch makes the problems
> > disappear completely, as far as we can tell.
> 
> The sig-hack patch is brutal in the sense that it was really unclean and
> not the right way to do it. :).
> 
> > 
> > So, I was asked by people, "Why does it work on NFS, and not on PVFS?"
> > I got to looking at the NFS client code, and as far as I can tell
> > (please correct me if I'm wrong), NFS doesn't allow itself to be
> > interrupted by signals by default, unless mounted with the intr flag.
> 
> True. SIGKILL is allowed irrespective of intr option or not.
> SIGINT, SIGQUIT are additional signals that can interrupt a process if
> intr option was provided. I dont think any other signal affects any of the
> network-part of NFS operations.
> 
> > behaviour with regard to signal interruption.  Attached is a sample
> > patch (WARNING: not tested! Very well may not work at all! Only intended
> > as an example of what I'm suggesting!) which implements a new parameter
> > to the pvfs.o kernel module, 'intr=N' (where N is 0 or 1, defaulting to
> > 0).  check_for_signal_to_kpvfsd() in kpvfsdev.c was modified to check
> > the value of this flag, and use the following logic:
> > 
> > * Always allow itself to be interrupted by SIGKILL, SIGTERM, and SIGINT
> > * If pvfs_intr is set to 1, allow itself to be interrupted by any
> >   signal.
> > * If pvfs_intr is set to 0, simply dequeue the signal and go on with
> >   life.
> 
> I sort of modified your ideas a little bit and attaching a patch which
> (hopefully ;-) ) does the following,
> a) intr is made a mount-time option and is part of pvfs_super's flags
> instead of the module load time option.
> b) Added code to set and reset a process's signal mask in [k]pvfsdev.c
> c) support infrastructure for passing mount-time flags to the upcalls.
> c) As soon as we enter pvfsdev_enqueue(), we check to see if we have
> mounted with intr- option and if not we mask all signals except SIGKILL,
> SIGINT, SIGQUIT. If it was mounted with intr, then we dont do any masking
> and the process can be interrupted by any signal.
> Note that we need to reset the process's old signal mask just before
> every possible return statement of pvfsdev_enqueue().
> d) A few minor function name changes (they were misnomers earlier) and
> print statement cleanups.
> 
> So the semantics dictated by this patch are summarized below
> a) FS mounted with intr: pvfs operations can be interrupted by any signal
> (current CVS behaviour I  think)
> b) FS mounted without intr: pvfs operations can be interrupted only by
> SIGKILL, SIGINT, SIGQUIT.
> 
> I dont know how to test whether this patch's functionality works or not,
> since I was never able to reproduce the behaviour that you got. It would
> be great if you try running your app on a PVFS volume mounted with
> "intr" and without that option and see what the behaviour is.  So in your
> case, I would expect that a default mount should cause the app to run
> successfully, and one with an intr-mount would fail the way it used to.
> 
> Suggestions/comments welcome.
> 
> Thanks,
> Murali
>