On (24/12/14 07:09), Jeff Layton wrote:
> > +void fuse_check_timeout(struct work_struct *work)
> > +{
> > + struct delayed_work *dwork = to_delayed_work(work);
> > + struct fuse_conn *fc = container_of(dwork, struct fuse_conn,
> > + timeout.work);
> > + struct fuse_iqueue *fiq = &fc->iq;
> > + struct fuse_req *req;
> > + struct fuse_dev *fud;
> > + struct fuse_pqueue *fpq;
> > + bool expired = false;
> > + int i;
> > +
[..]
> > +
> > +fpq_abort:
> > + spin_unlock(&fpq->lock);
> > + spin_unlock(&fc->lock);
> > +abort_conn:
> > + fuse_abort_conn(fc);
> > +}
> > +
> > @@ -2308,6 +2388,9 @@ void fuse_abort_conn(struct fuse_conn *fc)
> > spin_unlock(&fc->lock);
> >
> > end_requests(&to_end);
> > +
> > + if (fc->timeout.req_timeout)
> > + cancel_delayed_work(&fc->timeout.work);
>
> As Sergey pointed out, this should be a cancel_delayed_work_sync().
My worry here is that fuse_abort_conn() can also be called from the
deferred work handler, I'm not sure if we can cancel_delayed_work_sync()
from within the same WQ context, sounds deadlock-ish:
WQ -> fuse_check_timeout() -> fuse_abort_conn() -> cancel_delayed_work_sync()
When fuse_abort_conn() is called from somewhere else (umount, etc.) then
we can safely sync(), but fuse_check_timeout() is different.
Maybe fuse_abort_conn() can become __fuse_abort_conn(), which
fuse_check_timeout() will call directly, for the rest fuse_abort_conn()
can be something like:
static void __fuse_abort_conn()
{
....
}
void fuse_abort_conn()
{
cancel_delayed_work_sync()
__fuse_abort_conn();
}
