Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
MIME-Version: 1.0
References: <CA+fyA4RABYNPZZSk9+9U51u53kbSzqgwdi1KDDGRxXi8q5TtxQ@mail.gmail.com>
 <ZSm9zeepuZ94A-ZI@google.com>
In-Reply-To: <ZSm9zeepuZ94A-ZI@google.com>
From:   John Salamon <salamonj9@gmail.com>
Date:   Sat, 14 Oct 2023 17:02:35 +1030
Message-ID: <CA+fyA4QD6ojC7NMaYigK2xvHt0cpEPY3daGx01Hh3NVHV1sfMQ@mail.gmail.com>
Subject: Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when
 allocation is required
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc:     rydberg@bitmath.org, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Dmitry,

Thank you for the clarification.

> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.

In that case, would you consider accepting a patch adding a sentence
or two explaining that handlers will need to keep track of effect IDs
in this manner? Either in uinput documentation or just in the comment
describing how to implement upload_effect() in uinput.h. Something
along the lines of "Effects received here will never have an ID of -1.
Handlers of these effects must recognize previously unseen effect IDs
to know when a new effect has been allocated.".

Cheers,
John


On Sat, Oct 14, 2023 at 8:29=E2=80=AFAM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi John,
>
> On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> > Currently the "fake" input events generated by uinput in response to
> > effect uploads will return an effect with an id that has already been
> > handled by input_ff_upload in ff-core.c, which can modify the effect
> > id. This causes a problem specifically when the effect originally
> > uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> > userspace code handling UI_FF_UPLOAD receives an effect with an id
> > other than -1, and therefore will not know an allocation was
> > requested.
>
> The kernel never changes ID of an existing effect, the only time ID is
> changed is when userspace indicates that a new effect should be created
> by setting effect ID to -1.
>
> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.
>
> >
> > I notice that the "old" field on the ff_effect struct is set to NULL
> > when the -1 id is changed (in input_ff_upload), which can serve as a
> > flag that an allocation was requested. If it is the intention is that
> > uinput users check if old =3D=3D NULL to know when allocations are need=
ed
> > I think uinput documentation should describe this.
>
> No, not really, as explained above.
>
> >
> > I first noticed this using python-evdev, see my issue report here:
> > https://github.com/gvalkov/python-evdev/issues/199
>
> Thanks.
>
> --
> Dmitry