| Message ID | 20240724071156.97188-3-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: Add timeout support for fuse connection | expand |
On Wed, Jul 24, 2024 at 12:12 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > In our experience with fuse.hdfs, we encountered a challenge where, if the > HDFS server encounters an issue, the fuse.hdfs daemon—responsible for > sending requests to the HDFS server—can get stuck indefinitely. > Consequently, access to the fuse.hdfs directory becomes unresponsive. > The current workaround involves manually aborting the fuse connection, > which is unreliable in automatically addressing the abnormal connection > issue. To alleviate this pain point, we have implemented a timeout > mechanism that automatically handles such abnormal cases, thereby > streamlining the process and enhancing reliability. > > The timeout value is configurable by the user, allowing them to tailor it > according to their specific workload requirements. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Hi Yafang, There was a similar thread/conversation about timeouts started in this link from last week https://lore.kernel.org/linux-fsdevel/20240717213458.1613347-1-joannelkoong@gmail.com/#t The core idea is the same but also handles cleanup for requests that time out, to avoid memory leaks in cases where the server never replies to the request. For v2, I am going to add timeouts for background requests as well. Thanks, Joanne > --- > fs/fuse/dev.c | 57 +++++++++++++++++++++++++++++++++++++++++------- > fs/fuse/fuse_i.h | 2 ++ > 2 files changed, 51 insertions(+), 8 deletions(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 9eb191b5c4de..ff9c55bcfb3d 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -369,10 +369,27 @@ static void request_wait_answer(struct fuse_req *req) > > if (!fc->no_interrupt) { > /* Any signal may interrupt this */ > - err = wait_event_interruptible(req->waitq, > - test_bit(FR_FINISHED, &req->flags)); > - if (!err) > - return; > + if (!fc->timeout) { > + err = wait_event_interruptible(req->waitq, > + test_bit(FR_FINISHED, &req->flags)); > + if (!err) > + return; > + } else { > + err = wait_event_interruptible_timeout(req->waitq, > + test_bit(FR_FINISHED, &req->flags), > + (long)fc->timeout * HZ); > + if (err > 0) > + return; > + > + /* timeout */ > + if (!err) { > + req->out.h.error = -EAGAIN; > + set_bit(FR_TIMEOUT, &req->flags); > + /* matches barrier in fuse_dev_do_write() */ > + smp_mb__after_atomic(); > + return; > + } > + } > > set_bit(FR_INTERRUPTED, &req->flags); > /* matches barrier in fuse_dev_do_read() */ > @@ -383,10 +400,27 @@ static void request_wait_answer(struct fuse_req *req) > > if (!test_bit(FR_FORCE, &req->flags)) { > /* Only fatal signals may interrupt this */ > - err = wait_event_killable(req->waitq, > - test_bit(FR_FINISHED, &req->flags)); > - if (!err) > - return; > + if (!fc->timeout) { > + err = wait_event_killable(req->waitq, > + test_bit(FR_FINISHED, &req->flags)); > + if (!err) > + return; > + } else { > + err = wait_event_killable_timeout(req->waitq, > + test_bit(FR_FINISHED, &req->flags), > + (long)fc->timeout * HZ); > + if (err > 0) > + return; > + > + /* timeout */ > + if (!err) { > + req->out.h.error = -EAGAIN; > + set_bit(FR_TIMEOUT, &req->flags); > + /* matches barrier in fuse_dev_do_write() */ > + smp_mb__after_atomic(); > + return; > + } > + } > > spin_lock(&fiq->lock); > /* Request is not yet in userspace, bail out */ > @@ -1951,6 +1985,13 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud, > goto copy_finish; > } > > + /* matches barrier in request_wait_answer() */ > + smp_mb__after_atomic(); > + if (test_and_clear_bit(FR_TIMEOUT, &req->flags)) { > + spin_unlock(&fpq->lock); > + goto copy_finish; > + } > + > /* Is it an interrupt reply ID? */ > if (oh.unique & FUSE_INT_REQ_BIT) { > __fuse_get_request(req); > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index 367601bf7285..c1467eb8c2e9 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -375,6 +375,7 @@ struct fuse_io_priv { > * FR_FINISHED: request is finished > * FR_PRIVATE: request is on private list > * FR_ASYNC: request is asynchronous > + * FR_TIMEOUT: request is timeout > */ > enum fuse_req_flag { > FR_ISREPLY, > @@ -389,6 +390,7 @@ enum fuse_req_flag { > FR_FINISHED, > FR_PRIVATE, > FR_ASYNC, > + FR_TIMEOUT, > }; > > /** > -- > 2.43.5 > >
On Thu, Jul 25, 2024 at 1:09 AM Joanne Koong <joannelkoong@gmail.com> wrote: > > On Wed, Jul 24, 2024 at 12:12 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > In our experience with fuse.hdfs, we encountered a challenge where, if the > > HDFS server encounters an issue, the fuse.hdfs daemon—responsible for > > sending requests to the HDFS server—can get stuck indefinitely. > > Consequently, access to the fuse.hdfs directory becomes unresponsive. > > The current workaround involves manually aborting the fuse connection, > > which is unreliable in automatically addressing the abnormal connection > > issue. To alleviate this pain point, we have implemented a timeout > > mechanism that automatically handles such abnormal cases, thereby > > streamlining the process and enhancing reliability. > > > > The timeout value is configurable by the user, allowing them to tailor it > > according to their specific workload requirements. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Hi Yafang, > > There was a similar thread/conversation about timeouts started in this > link from last week > https://lore.kernel.org/linux-fsdevel/20240717213458.1613347-1-joannelkoong@gmail.com/#t > I am not currently subscribed to linux-fsdevel, so I missed your patch. Thanks for your information. I will test your patch. > The core idea is the same but also handles cleanup for requests that > time out, to avoid memory leaks in cases where the server never > replies to the request. For v2, I am going to add timeouts for > background requests as well. Please CC me if you send new versions.
On Wed, Jul 24, 2024 at 7:07 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > On Thu, Jul 25, 2024 at 1:09 AM Joanne Koong <joannelkoong@gmail.com> wrote: > > > > On Wed, Jul 24, 2024 at 12:12 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > > > > > In our experience with fuse.hdfs, we encountered a challenge where, if the > > > HDFS server encounters an issue, the fuse.hdfs daemon—responsible for > > > sending requests to the HDFS server—can get stuck indefinitely. > > > Consequently, access to the fuse.hdfs directory becomes unresponsive. > > > The current workaround involves manually aborting the fuse connection, > > > which is unreliable in automatically addressing the abnormal connection > > > issue. To alleviate this pain point, we have implemented a timeout > > > mechanism that automatically handles such abnormal cases, thereby > > > streamlining the process and enhancing reliability. > > > > > > The timeout value is configurable by the user, allowing them to tailor it > > > according to their specific workload requirements. > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > > Hi Yafang, > > > > There was a similar thread/conversation about timeouts started in this > > link from last week > > https://lore.kernel.org/linux-fsdevel/20240717213458.1613347-1-joannelkoong@gmail.com/#t > > > > I am not currently subscribed to linux-fsdevel, so I missed your patch. > Thanks for your information. I will test your patch. > > > The core idea is the same but also handles cleanup for requests that > > time out, to avoid memory leaks in cases where the server never > > replies to the request. For v2, I am going to add timeouts for > > background requests as well. > > Please CC me if you send new versions. Will do. I'll make sure you are cc-ed. Thanks, Joanne > > -- > Regards > Yafang
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 9eb191b5c4de..ff9c55bcfb3d 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -369,10 +369,27 @@ static void request_wait_answer(struct fuse_req *req) if (!fc->no_interrupt) { /* Any signal may interrupt this */ - err = wait_event_interruptible(req->waitq, - test_bit(FR_FINISHED, &req->flags)); - if (!err) - return; + if (!fc->timeout) { + err = wait_event_interruptible(req->waitq, + test_bit(FR_FINISHED, &req->flags)); + if (!err) + return; + } else { + err = wait_event_interruptible_timeout(req->waitq, + test_bit(FR_FINISHED, &req->flags), + (long)fc->timeout * HZ); + if (err > 0) + return; + + /* timeout */ + if (!err) { + req->out.h.error = -EAGAIN; + set_bit(FR_TIMEOUT, &req->flags); + /* matches barrier in fuse_dev_do_write() */ + smp_mb__after_atomic(); + return; + } + } set_bit(FR_INTERRUPTED, &req->flags); /* matches barrier in fuse_dev_do_read() */ @@ -383,10 +400,27 @@ static void request_wait_answer(struct fuse_req *req) if (!test_bit(FR_FORCE, &req->flags)) { /* Only fatal signals may interrupt this */ - err = wait_event_killable(req->waitq, - test_bit(FR_FINISHED, &req->flags)); - if (!err) - return; + if (!fc->timeout) { + err = wait_event_killable(req->waitq, + test_bit(FR_FINISHED, &req->flags)); + if (!err) + return; + } else { + err = wait_event_killable_timeout(req->waitq, + test_bit(FR_FINISHED, &req->flags), + (long)fc->timeout * HZ); + if (err > 0) + return; + + /* timeout */ + if (!err) { + req->out.h.error = -EAGAIN; + set_bit(FR_TIMEOUT, &req->flags); + /* matches barrier in fuse_dev_do_write() */ + smp_mb__after_atomic(); + return; + } + } spin_lock(&fiq->lock); /* Request is not yet in userspace, bail out */ @@ -1951,6 +1985,13 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud, goto copy_finish; } + /* matches barrier in request_wait_answer() */ + smp_mb__after_atomic(); + if (test_and_clear_bit(FR_TIMEOUT, &req->flags)) { + spin_unlock(&fpq->lock); + goto copy_finish; + } + /* Is it an interrupt reply ID? */ if (oh.unique & FUSE_INT_REQ_BIT) { __fuse_get_request(req); diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 367601bf7285..c1467eb8c2e9 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -375,6 +375,7 @@ struct fuse_io_priv { * FR_FINISHED: request is finished * FR_PRIVATE: request is on private list * FR_ASYNC: request is asynchronous + * FR_TIMEOUT: request is timeout */ enum fuse_req_flag { FR_ISREPLY, @@ -389,6 +390,7 @@ enum fuse_req_flag { FR_FINISHED, FR_PRIVATE, FR_ASYNC, + FR_TIMEOUT, }; /**
In our experience with fuse.hdfs, we encountered a challenge where, if the HDFS server encounters an issue, the fuse.hdfs daemon—responsible for sending requests to the HDFS server—can get stuck indefinitely. Consequently, access to the fuse.hdfs directory becomes unresponsive. The current workaround involves manually aborting the fuse connection, which is unreliable in automatically addressing the abnormal connection issue. To alleviate this pain point, we have implemented a timeout mechanism that automatically handles such abnormal cases, thereby streamlining the process and enhancing reliability. The timeout value is configurable by the user, allowing them to tailor it according to their specific workload requirements. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- fs/fuse/dev.c | 57 +++++++++++++++++++++++++++++++++++++++++------- fs/fuse/fuse_i.h | 2 ++ 2 files changed, 51 insertions(+), 8 deletions(-)