On Fri, Dec 13, 2024 at 10:53 PM Sergey Senozhatsky
<senozhatsky@xxxxxxxxxxxx> wrote:
>
> On (24/12/13 18:28), Joanne Koong 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;
> > +
> > + spin_lock(&fiq->lock);
> > + req = list_first_entry_or_null(&fiq->pending, struct fuse_req, list);
> > + if (req)
> > + expired = request_expired(fc, req);
>
> A nit: you can factor these out into a small helper
>
> static bool request_expired(struct fuse_conn *fc, struct list_head *list)
> {
> struct fuse_req *req;
>
> req = list_first_entry_or_null(list, struct fuse_req, list);
> if (!req)
> return false;
> return time_after(jiffies, req->create_time + fuse_watchdog_timeout());
> }
>
> and just call it passing the corresponding list pointer
>
> abort = request_expired(fc, &fiq->pending);
>
> kinda makes the function look less busy.
Good idea! I'll do this refactoring as part of v11.
>
> [..]
> > @@ -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);
>
> When fuse_abort_conn() is called not from fuse_check_timeout(), but from
> somewhere else, should this use cancel_delayed_work_sync()?
I left a comment about this under the reply to Jeff.
Thanks,
Joanne
