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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
