lxcfs: : Don't register .poll at all unless --enable-psi-poll is passed

[tomponline]

Without .poll in lxcfs_ops, the kernel handles poll entirely in-kernel (returning DEFAULT_POLLMASK without ever contacting userspace), so there is no FUSE round-trip and no D state when LXCFS is suspended.

However this only fixes the problem as long as --enable-psi-poll is not passed.

Partial fix for canonical/lxd#17983

[tomponline]

Hi @mihalicyn what are your thoughts on this one?

I realise this is only really a band-aid solution, so I wondered if you had a more full-proof solution?

Thanks!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[mihalicyn]

Hi Tom,

thank you for the patch. WDYT about the following:

diff --git a/src/lxcfs.c b/src/lxcfs.c
index 9ecdffd..5fb3360 100644
--- a/src/lxcfs.c
+++ b/src/lxcfs.c
@@ -540,9 +540,24 @@ int lxcfs_write(const char *path, const char *buf, size_t size, off_t offset,
 int lxcfs_poll(const char *path, struct fuse_file_info *fi,
               struct fuse_pollhandle *ph, unsigned *reventsp)
 {
+       struct fuse_context *fc = fuse_get_context();
+       struct lxcfs_opts *opts = fc->private_data;
        int ret;
        enum lxcfs_virt_t type;
 
+       /*
+        * Note, that if fuse daemon returns ENOSYS once, then
+        * kernel caches this result in fm->fc->no_poll and never
+        * sends FUSE_POLL again. It is not a problem for us now,
+        * because we only need poll() for PSI.
+        *
+        * See #726 on GitHub.
+        */
+       if (!opts->psi_poll_on) {
+               fuse_pollhandle_destroy(ph);
+               return -ENOSYS;
+       }
+
        type = file_info_type(fi);
 
        if (LXCFS_TYPE_PROC(type)) {

[mihalicyn]

I realise this is only really a band-aid solution, so I wondered if you had a more full-proof solution?

At this point no. I need to dive into suspend code to figure out why it can't just cancel request. A long ago I've added this #594 to deal with cases like this one and avoid D-states when possible.

[tomponline]

Hi Tom,

thank you for the patch. WDYT about the following:

diff --git a/src/lxcfs.c b/src/lxcfs.c
index 9ecdffd..5fb3360 100644
--- a/src/lxcfs.c
+++ b/src/lxcfs.c
@@ -540,9 +540,24 @@ int lxcfs_write(const char *path, const char *buf, size_t size, off_t offset,
 int lxcfs_poll(const char *path, struct fuse_file_info *fi,
               struct fuse_pollhandle *ph, unsigned *reventsp)
 {
+       struct fuse_context *fc = fuse_get_context();
+       struct lxcfs_opts *opts = fc->private_data;
        int ret;
        enum lxcfs_virt_t type;
 
+       /*
+        * Note, that if fuse daemon returns ENOSYS once, then
+        * kernel caches this result in fm->fc->no_poll and never
+        * sends FUSE_POLL again. It is not a problem for us now,
+        * because we only need poll() for PSI.
+        *
+        * See #726 on GitHub.
+        */
+       if (!opts->psi_poll_on) {
+               fuse_pollhandle_destroy(ph);
+               return -ENOSYS;
+       }
+
        type = file_info_type(fi);
 
        if (LXCFS_TYPE_PROC(type)) {

Thanks @mihalicyn ! This works well too, would you like me to update the PR or do you want to push your own commit to it?

Thanks

[mihalicyn]

Hi Tom,
thank you for the patch. WDYT about the following:

diff --git a/src/lxcfs.c b/src/lxcfs.c
index 9ecdffd..5fb3360 100644
--- a/src/lxcfs.c
+++ b/src/lxcfs.c
@@ -540,9 +540,24 @@ int lxcfs_write(const char *path, const char *buf, size_t size, off_t offset,
 int lxcfs_poll(const char *path, struct fuse_file_info *fi,
               struct fuse_pollhandle *ph, unsigned *reventsp)
 {
+       struct fuse_context *fc = fuse_get_context();
+       struct lxcfs_opts *opts = fc->private_data;
        int ret;
        enum lxcfs_virt_t type;
 
+       /*
+        * Note, that if fuse daemon returns ENOSYS once, then
+        * kernel caches this result in fm->fc->no_poll and never
+        * sends FUSE_POLL again. It is not a problem for us now,
+        * because we only need poll() for PSI.
+        *
+        * See #726 on GitHub.
+        */
+       if (!opts->psi_poll_on) {
+               fuse_pollhandle_destroy(ph);
+               return -ENOSYS;
+       }
+
        type = file_info_type(fi);
 
        if (LXCFS_TYPE_PROC(type)) {

Thanks @mihalicyn ! This works well too, would you like me to update the PR or do you want to push your own commit to it?

Thanks

yes, please update the PR. You don't need to mention me in the commit anyhow, it's just my review suggestion ;-)

[tomponline]

I realised my local branch was master rather than main so have opened a new PR over here #727