linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* BUG: ff_effects lost after a fork
@ 2019-11-27 10:10 Mathieu Maret
  2020-04-21 18:53 ` Brendan Shanks
  0 siblings, 1 reply; 3+ messages in thread
From: Mathieu Maret @ 2019-11-27 10:10 UTC (permalink / raw)
  To: dmitry.torokhov, rydberg, linux-input; +Cc: linux-kernel, mmaret

Hi,

I'm using evdev for vibrator interface.
I can register ff_effect and play them.
But, if I do any kind of fork, all the effects are flush and cannot be
used.

You can find, below, an example of such a program.
From some trace have put in the kernel, it's seems that at the end of
the system() call, evdev_flush get called.

evdev_flush() will call flush_effects() that will remove all the
registered effects.

I've only one device with vibrator and it's a imx6 4.1.15 kernel. But
code looks the same that in linus master that why I'm posting it here. I
hope that it will not waste people time

For the moment, I'm using this nasty workaround:

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e578a75..6e6002d 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -415,6 +415,8 @@ static int evdev_release(struct inode *inode, struct file *file)
 	struct evdev_client *client = file->private_data;
 	struct evdev *evdev = client->evdev;

+	evdev_flush(file, NULL);
+
 	mutex_lock(&evdev->mutex);
 	evdev_ungrab(evdev, client);
 	mutex_unlock(&evdev->mutex);
@@ -1107,7 +1109,7 @@ static const struct file_operations evdev_fops = {
 	.compat_ioctl	= evdev_ioctl_compat,
 #endif
 	.fasync		= evdev_fasync,
-	.flush		= evdev_flush,
+//	.flush		= evdev_flush,
 	.llseek		= no_llseek,
 };


* C program example


#include <errno.h>
#include <fcntl.h>
#include <linux/input.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdlib.h>

#define DEV_PATH "/dev/input/event1"

int main(int argc, char *argv[])
{
    int fd = open(DEV_PATH, O_RDWR);
    if (fd < 0) {
        printf("Cannot open " DEV_PATH);
    }

    // Register an effect
    struct ff_effect effects;
    memset(&effects, 0, sizeof(effects));
    effects.type                      = FF_RUMBLE;
    effects.id                        = -1;
    effects.u.rumble.strong_magnitude = 0x8000;
    effects.u.rumble.weak_magnitude   = 0;
    effects.replay.length             = 1000;
    effects.replay.delay              = 0;

    if (ioctl(fd, EVIOCSFF, &effects) < 0) {
        printf("Cannot upload effect %s\n", strerror(errno));
        return -1;
    }

    // Play this effect
    struct input_event input;
    memset(&input, 0, sizeof(input));
    input.type  = EV_FF;
    input.code  = effects.id;
    input.value = 1;
    if (write(fd, &input, sizeof(input)) != sizeof(input)) {
        printf("Cannot write %s\n", strerror(errno));
        return -1;
    }

    printf("Forking\n");
    system("sleep 1"); // Comment this line to have the second effect
    played

    // Play effect again : Nothing is played
    memset(&input, 0, sizeof(input));
    input.type  = EV_FF;
    input.code  = effects.id;
    input.value = 1;
    if (write(fd, &input, sizeof(input)) != sizeof(input)) {
        printf("Cannot write %s\n", strerror(errno));
        return -1;
    }


    close(fd);
    return 0;
}


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: BUG: ff_effects lost after a fork
  2019-11-27 10:10 BUG: ff_effects lost after a fork Mathieu Maret
@ 2020-04-21 18:53 ` Brendan Shanks
  2020-04-21 19:28   ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Brendan Shanks @ 2020-04-21 18:53 UTC (permalink / raw)
  To: linux-input; +Cc: dmitry.torokhov, rydberg, linux-kernel, Mathieu Maret, mmaret


> On Nov 27, 2019, at 2:10 AM, Mathieu Maret <mathieu.maret@gmail.com> wrote:
> 
> Hi,
> 
> I'm using evdev for vibrator interface.
> I can register ff_effect and play them.
> But, if I do any kind of fork, all the effects are flush and cannot be
> used.
> 
> You can find, below, an example of such a program.
> From some trace have put in the kernel, it's seems that at the end of
> the system() call, evdev_flush get called.
> 
> evdev_flush() will call flush_effects() that will remove all the
> registered effects.
> 
> I've only one device with vibrator and it's a imx6 4.1.15 kernel. But
> code looks the same that in linus master that why I'm posting it here. I
> hope that it will not waste people time
> 

Hi everyone,

I’m also hitting this bug with games that use force-feedback steering wheels under Wine/Proton. It typically shows up as EVIOCSFF ioctls failing with EINVAL, since all the effects were unexpectedly flushed.

The problem is that input_ff_flush() is called whenever a file descriptor is closed, but there can be multiple descriptors open to the same file description (through fork(), dup(), etc). input_ff_flush() removes all effects added by that file description, which the users of the other descriptors certainly don't expect.

As for the fix, maybe fd_ops->flush() shouldn’t be implemented at all?
In the current design, effects “belong” to a file description (a struct file *), not a descriptor. This seems sensible to me: a process could open a device, upload an effect, then fork(), and it makes sense that the child would have full control of the effects created by the parent. It seems to me like nothing should be done when a descriptor is closed, and input_ff_flush() should be called only when the whole struct file is released.

I’ve attached a modified copy of Mathieu’s code, which reproduces the problem for me with a Logitech G25 steering wheel.

Brendan Shanks
CodeWeavers




#include <errno.h>
#include <fcntl.h>
#include <linux/input.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdlib.h>

#define DEV_PATH "/dev/input/event10"

int main(int argc, char *argv[])
{
    int fd = open(DEV_PATH, O_RDWR);
    if (fd < 0) {
        printf("Cannot open " DEV_PATH);
    }
    // Register an effect
    struct ff_effect effects;
    memset(&effects, 0, sizeof(effects));
    effects.type                      = FF_CONSTANT;
    effects.id                        = -1;
    effects.replay.length             = 1000;
    effects.replay.delay              = 0;
    if (ioctl(fd, EVIOCSFF, &effects) < 0) {
        printf("Cannot upload effect %s\n", strerror(errno));
        return -1;
    }

    int fd2 = dup(fd);
    close(fd2);

    // ioctl fails with EINVAL
    if (ioctl(fd, EVIOCSFF, &effects) < 0) {
        printf("Cannot upload effect %s\n", strerror(errno));
        return -1;
    }

    close(fd);
    return 0;
}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: BUG: ff_effects lost after a fork
  2020-04-21 18:53 ` Brendan Shanks
@ 2020-04-21 19:28   ` Dmitry Torokhov
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2020-04-21 19:28 UTC (permalink / raw)
  To: Brendan Shanks; +Cc: linux-input, rydberg, linux-kernel, Mathieu Maret, mmaret

On Tue, Apr 21, 2020 at 11:53:19AM -0700, Brendan Shanks wrote:
> 
> > On Nov 27, 2019, at 2:10 AM, Mathieu Maret <mathieu.maret@gmail.com>
> > wrote:
> > 
> > Hi,
> > 
> > I'm using evdev for vibrator interface.  I can register ff_effect
> > and play them.  But, if I do any kind of fork, all the effects are
> > flush and cannot be used.
> > 
> > You can find, below, an example of such a program.  From some trace
> > have put in the kernel, it's seems that at the end of the system()
> > call, evdev_flush get called.
> > 
> > evdev_flush() will call flush_effects() that will remove all the
> > registered effects.
> > 
> > I've only one device with vibrator and it's a imx6 4.1.15 kernel.
> > But code looks the same that in linus master that why I'm posting it
> > here. I hope that it will not waste people time
> > 
> 
> Hi everyone,
> 
> I’m also hitting this bug with games that use force-feedback steering
> wheels under Wine/Proton. It typically shows up as EVIOCSFF ioctls
> failing with EINVAL, since all the effects were unexpectedly flushed.
> 
> The problem is that input_ff_flush() is called whenever a file
> descriptor is closed, but there can be multiple descriptors open to
> the same file description (through fork(), dup(), etc).
> input_ff_flush() removes all effects added by that file description,
> which the users of the other descriptors certainly don't expect.
> 
> As for the fix, maybe fd_ops->flush() shouldn’t be implemented at all?
> In the current design, effects “belong” to a file description (a
> struct file *), not a descriptor. This seems sensible to me: a process
> could open a device, upload an effect, then fork(), and it makes sense
> that the child would have full control of the effects created by the
> parent. It seems to me like nothing should be done when a descriptor
> is closed, and input_ff_flush() should be called only when the whole
> struct file is released.
> 

Yes, I agree, we should drop setting fops->flush and move the code into
evdev_release(). This will ensure that the effects are stopped and
erased once file is closed, but will allow passing the descriptors
around.

Can I get a real patch for this please?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-21 19:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 10:10 BUG: ff_effects lost after a fork Mathieu Maret
2020-04-21 18:53 ` Brendan Shanks
2020-04-21 19:28   ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).