Re: [PATCH 4/9] nfs: add a freezable_schedule_timeout_unsafe() and use it when waiting to retry LOCK

[Jeff Layton <jlayton@redhat.com>]

On Thu, 2016-09-08 at 14:20 -0400, Anna Schumaker wrote:
> Hi Jeff,
> 
> Can you CC the freezer folks on this patch during the second
> posting?  They'll probably need to at send an ACK for the changes to
> their include file.
> 
> Thanks,
> Anna
> 

Will do...

OTOH, now that I look, I'm not sure we really need a *_unsafe variant
here at all.

File locking is somewhat different in that we're expected to sleep in
the kernel, and I don't think we hold any kernel locks that are the
usual worry here (i_rwsem and the like). So we might can just use
freezable_schedule_timeout here.

I'll see if I can experiment with that before the next posting. It'd be
nice to eliminate one of the unsafe sleep callsites.

> On 09/06/2016 11:12 AM, Jeff Layton wrote:
> > 
> > We actually want to use TASK_INTERRUPTIBLE sleeps here. Once the
> > task
> > wakes up, if there is a signal pending then we'll be returning an
> > error
> > anyway. So, we might as well wake up immediately for non-fatal
> > signals
> > as well. That allows us to return to userland more quickly in that
> > case,
> > but won't change the error that userland sees.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/nfs/nfs4proc.c       |  3 ++-
> >  include/linux/freezer.h | 13 +++++++++++++
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index e3bf95369daf..e9232d71bc64 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5537,7 +5537,8 @@ int nfs4_proc_delegreturn(struct inode
> > *inode, struct rpc_cred *cred, const nfs4
> >  static unsigned long
> >  nfs4_set_lock_task_retry(unsigned long timeout)
> >  {
> > -	freezable_schedule_timeout_killable_unsafe(timeout);
> > +	set_current_state(TASK_INTERRUPTIBLE);
> > +	freezable_schedule_timeout_unsafe(timeout);
> >  	timeout <<= 1;
> >  	if (timeout > NFS4_LOCK_MAXTIMEOUT)
> >  		return NFS4_LOCK_MAXTIMEOUT;
> > diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> > index dd03e837ebb7..fe31601e7f55 100644
> > --- a/include/linux/freezer.h
> > +++ b/include/linux/freezer.h
> > @@ -197,6 +197,19 @@ static inline long
> > freezable_schedule_timeout(long timeout)
> >   * Like schedule_timeout_interruptible(), but should not block the
> > freezer.  Do not
> >   * call this with locks held.
> >   */
> > +static inline long freezable_schedule_timeout_unsafe(long timeout)
> > +{
> > +	long __retval;
> > +	freezer_do_not_count();
> > +	__retval = schedule_timeout(timeout);
> > +	freezer_count_unsafe();
> > +	return __retval;
> > +}
> > +
> > +/*
> > + * Like schedule_timeout_interruptible(), but should not block the
> > freezer.  Do not
> > + * call this with locks held.
> > + */
> >  static inline long freezable_schedule_timeout_interruptible(long
> > timeout)
> >  {
> >  	long __retval;
> > 
> 

-- 
Jeff Layton <jlayton@redhat.com>