[Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr. Dmitry and Linux Community,

I am Anshul Garg working on Linux Kernel from last 2 years .

I have one query regarding event filter mechanism in input subsystem.
Can you please help in answering my query as below:

==============================
=======================
In function input_pass_values in input.c file , input core sends all
events to each handler associated with

the input device ,

rcu_read_lock();

handle = rcu_dereference(dev->grab);
if (handle) {
count = input_to_handler(handle, vals, count);
} else {
list_for_each_entry_rcu(handle, &dev->h_list, d_node)
if (handle->open)
count = input_to_handler(handle, vals, count);
}

after this in input_to_handler function events are filtered and sent
to the handler.

for (v = vals; v != vals + count; v++) {
if (handler->filter &&
   handler->filter(handle, v->type, v->code, v->value))
continue;
if (end != v)
*end = *v;
end++;
}



But as per previous event filter mechanism all the events should be
parsed from all
handlers after that remaining events should be sent to handlers list.

And in comments also its mentioned as

/*
 * Pass event first through all filters and then, if event has not been
 * filtered out, through all open handles.*/

So current approach to filter events seems to be incorrect.

Please help to clarify my query.


I have prepared one patch with new event filter mechanism which will
first filter all events from attached handlers then pass the remaining
events after being filtered
through all handlers unlike current implementation.

Once My query is resolved , I will send the patch.


Thanks
Anshul Garg
+91-9899777351

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Dmitry Torokhov]

Hi Anshul,

On Mon, Dec 22, 2014 at 10:36:09PM +0530, Anshul Garg wrote:
> In function input_pass_values in input.c file , input core sends all
> events to each handler associated with the input device ,
> 
> rcu_read_lock();
> 
> handle = rcu_dereference(dev->grab);
> if (handle) {
> count = input_to_handler(handle, vals, count);
> } else {
> list_for_each_entry_rcu(handle, &dev->h_list, d_node)
> if (handle->open)
> count = input_to_handler(handle, vals, count);
> }
> 
> after this in input_to_handler function events are filtered and sent
> to the handler.
> 
> for (v = vals; v != vals + count; v++) {
> if (handler->filter &&
>    handler->filter(handle, v->type, v->code, v->value))
> continue;
> if (end != v)
> *end = *v;
> end++;
> }
> 
> 
> 
> But as per previous event filter mechanism all the events should be
> parsed from all
> handlers after that remaining events should be sent to handlers list.
> 
> And in comments also its mentioned as
> 
> /*
>  * Pass event first through all filters and then, if event has not been
>  * filtered out, through all open handles.*/
> 
> So current approach to filter events seems to be incorrect.
> 
> Please help to clarify my query.

When you "grab" input device your handler gets exclusive access to all
events coming form it. Neither filer handlers nor regular input handlers
receive events from this device until you release it.

So I believe it works as intended.

Thanks.

-- 
Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr Dmitry ,

Thanks for the reply.

I understand that if some handler grabs the input device then all
events will sent
to that handler only.

My Concern is If No one has grabbed the input device then all events
should go to
all handlers after application of all filter handlers.

For example : If 5 handlers registered for the input device.
Now among these 5 handlers 2 are filter handlers and remaining 3 are regular
input handlers.

In this case as per current implementation we pass the events array to
each handler.
Handler does its filtering then sent to handler.

What i am proposing is first we have to pass input_value list to all
filter handlers
After filteration of events, we can send the remaining events (Some
events might be removed
after filter) to all handlers.

In case device is grabbed we will just send the events to handler
which grabbed to device.

I hope to hear from you soon.

Thanks

On Mon, Dec 22, 2014 at 10:56 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Anshul,
>
> On Mon, Dec 22, 2014 at 10:36:09PM +0530, Anshul Garg wrote:
>> In function input_pass_values in input.c file , input core sends all
>> events to each handler associated with the input device ,
>>
>> rcu_read_lock();
>>
>> handle = rcu_dereference(dev->grab);
>> if (handle) {
>> count = input_to_handler(handle, vals, count);
>> } else {
>> list_for_each_entry_rcu(handle, &dev->h_list, d_node)
>> if (handle->open)
>> count = input_to_handler(handle, vals, count);
>> }
>>
>> after this in input_to_handler function events are filtered and sent
>> to the handler.
>>
>> for (v = vals; v != vals + count; v++) {
>> if (handler->filter &&
>>    handler->filter(handle, v->type, v->code, v->value))
>> continue;
>> if (end != v)
>> *end = *v;
>> end++;
>> }
>>
>>
>>
>> But as per previous event filter mechanism all the events should be
>> parsed from all
>> handlers after that remaining events should be sent to handlers list.
>>
>> And in comments also its mentioned as
>>
>> /*
>>  * Pass event first through all filters and then, if event has not been
>>  * filtered out, through all open handles.*/
>>
>> So current approach to filter events seems to be incorrect.
>>
>> Please help to clarify my query.
>
> When you "grab" input device your handler gets exclusive access to all
> events coming form it. Neither filer handlers nor regular input handlers
> receive events from this device until you release it.
>
> So I believe it works as intended.
>
> Thanks.
>
> --
> Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr Dmitry ,

Thanks for the reply.

I understand that if some handler grabs the input device then all
events will sent to that handler only.

My Concern is If No handler has grabbed the input device then all events
should go to all handlers after application of all filter handlers on
input event
list ( As we have to check for each event whether that event can be filtered
or not).

For example : If 5 handlers registered for the input device and input device is
not grabbed.
Now among these 5 handlers 2 are filter handlers and remaining 3 are regular
input handlers.
So we have to filter the event list first after applying 2 filters
then send the remaining
events to all registered handler.

In this case as per current implementation we pass the events array to
each handler. Input Core does events filtering fr handler then send remaining
events to handler.

What i am proposing is first we have to pass input_value list to all
filter handlers
After filteration of events, we can send the remaining events (Some
events might be removed after applying filter) to all handlers.

In case device is grabbed we will just send the events to handler
which grabbed to device.

I hope to hear from you soon.

Thanks

On Mon, Dec 22, 2014 at 10:56 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Anshul,
>
> On Mon, Dec 22, 2014 at 10:36:09PM +0530, Anshul Garg wrote:
>> In function input_pass_values in input.c file , input core sends all
>> events to each handler associated with the input device ,
>>
>> rcu_read_lock();
>>
>> handle = rcu_dereference(dev->grab);
>> if (handle) {
>> count = input_to_handler(handle, vals, count);
>> } else {
>> list_for_each_entry_rcu(handle, &dev->h_list, d_node)
>> if (handle->open)
>> count = input_to_handler(handle, vals, count);
>> }
>>
>> after this in input_to_handler function events are filtered and sent
>> to the handler.
>>
>> for (v = vals; v != vals + count; v++) {
>> if (handler->filter &&
>>    handler->filter(handle, v->type, v->code, v->value))
>> continue;
>> if (end != v)
>> *end = *v;
>> end++;
>> }
>>
>>
>>
>> But as per previous event filter mechanism all the events should be
>> parsed from all
>> handlers after that remaining events should be sent to handlers list.
>>
>> And in comments also its mentioned as
>>
>> /*
>>  * Pass event first through all filters and then, if event has not been
>>  * filtered out, through all open handles.*/
>>
>> So current approach to filter events seems to be incorrect.
>>
>> Please help to clarify my query.
>
> When you "grab" input device your handler gets exclusive access to all
> events coming form it. Neither filer handlers nor regular input handlers
> receive events from this device until you release it.
>
> So I believe it works as intended.
>
> Thanks.
>
> --
> Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Dmitry Torokhov]

Hi Anshul,

On Tue, Dec 23, 2014 at 08:04:45PM +0530, Anshul Garg wrote:
> Dear Mr Dmitry ,
> 
> Thanks for the reply.
> 
> I understand that if some handler grabs the input device then all
> events will sent to that handler only.
> 
> My Concern is If No handler has grabbed the input device then all events
> should go to all handlers after application of all filter handlers on
> input event
> list ( As we have to check for each event whether that event can be filtered
> or not).
> 
> For example : If 5 handlers registered for the input device and input device is
> not grabbed.
> Now among these 5 handlers 2 are filter handlers and remaining 3 are regular
> input handlers.
> So we have to filter the event list first after applying 2 filters
> then send the remaining
> events to all registered handler.
> 
> In this case as per current implementation we pass the events array to
> each handler. Input Core does events filtering fr handler then send remaining
> events to handler.
> 
> What i am proposing is first we have to pass input_value list to all
> filter handlers
> After filteration of events, we can send the remaining events (Some
> events might be removed after applying filter) to all handlers.

As far as I can see that is exactly what happens: we first pass the even list
to filters, which may cause the list to contract - see input_to_handler() - and
then to regular handlers.

I think the tricky part is the fact that we deliberately put filters in the
head of the dev->h_list, and normal handlers are put in the tail. And we also
expect the input_handler to either implement ->filter() or ->event[s]()
callback, but not both.

In case I still misunderstand what the issue you are trying to point out -
please do post your patch and we can discuss the code.

Thanks! 

-- 
Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr Dmitry ,

Thanks a lot for the clarification.

I was in assumption that one handler can support both ->filter() and
->event[s]()
Callback.So that's why i have prepared the patch to first do the
filter then pass
the events.

Can you please tell me why current implementation doesn't expect handler can
have both callbacks?

I think input core should be generic to allow any type of handlers which can
support both filter and events callbacks.

Please help to answer above query as my patch is based on this pre-assumption
that one handler can support both callbacks .

If we really need to have support of such handlers in input core then
only my patch
is good.

Hope to hear from you soon :)

Thanks

Anshul Garg

On Wed, Dec 24, 2014 at 11:55 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Anshul,
>
> On Tue, Dec 23, 2014 at 08:04:45PM +0530, Anshul Garg wrote:
>> Dear Mr Dmitry ,
>>
>> Thanks for the reply.
>>
>> I understand that if some handler grabs the input device then all
>> events will sent to that handler only.
>>
>> My Concern is If No handler has grabbed the input device then all events
>> should go to all handlers after application of all filter handlers on
>> input event
>> list ( As we have to check for each event whether that event can be filtered
>> or not).
>>
>> For example : If 5 handlers registered for the input device and input device is
>> not grabbed.
>> Now among these 5 handlers 2 are filter handlers and remaining 3 are regular
>> input handlers.
>> So we have to filter the event list first after applying 2 filters
>> then send the remaining
>> events to all registered handler.
>>
>> In this case as per current implementation we pass the events array to
>> each handler. Input Core does events filtering fr handler then send remaining
>> events to handler.
>>
>> What i am proposing is first we have to pass input_value list to all
>> filter handlers
>> After filteration of events, we can send the remaining events (Some
>> events might be removed after applying filter) to all handlers.
>
> As far as I can see that is exactly what happens: we first pass the even list
> to filters, which may cause the list to contract - see input_to_handler() - and
> then to regular handlers.
>
> I think the tricky part is the fact that we deliberately put filters in the
> head of the dev->h_list, and normal handlers are put in the tail. And we also
> expect the input_handler to either implement ->filter() or ->event[s]()
> callback, but not both.
>
> In case I still misunderstand what the issue you are trying to point out -
> please do post your patch and we can discuss the code.
>
> Thanks!
>
> --
> Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Dmitry Torokhov]

Hi Anshul,

On Thu, Dec 25, 2014 at 11:11:06AM +0530, Anshul Garg wrote:
> Dear Mr Dmitry ,
> 
> Thanks a lot for the clarification.
> 
> I was in assumption that one handler can support both ->filter() and
> ->event[s]()
> Callback.So that's why i have prepared the patch to first do the
> filter then pass
> the events.
> 
> Can you please tell me why current implementation doesn't expect handler can
> have both callbacks?

Because it was something I saw no need for: filter already has all the
events so it can process them. If you really want to process events
again once all filters have run, you can register additional handler.

> 
> I think input core should be generic to allow any type of handlers which can
> support both filter and events callbacks.
> 
> Please help to answer above query as my patch is based on this pre-assumption
> that one handler can support both callbacks .
> 
> If we really need to have support of such handlers in input core then
> only my patch
> is good.

I think I would need a user for this feature before changing the code to
allow it.

Thanks.

-- 
Dmitry

Re: [Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr. Dmitry ,

Thanks a lot for clearing my doubts :)



On Sat, Dec 27, 2014 at 5:16 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Anshul,
>
> On Thu, Dec 25, 2014 at 11:11:06AM +0530, Anshul Garg wrote:
>> Dear Mr Dmitry ,
>>
>> Thanks a lot for the clarification.
>>
>> I was in assumption that one handler can support both ->filter() and
>> ->event[s]()
>> Callback.So that's why i have prepared the patch to first do the
>> filter then pass
>> the events.
>>
>> Can you please tell me why current implementation doesn't expect handler can
>> have both callbacks?
>
> Because it was something I saw no need for: filter already has all the
> events so it can process them. If you really want to process events
> again once all filters have run, you can register additional handler.
>
>>
>> I think input core should be generic to allow any type of handlers which can
>> support both filter and events callbacks.
>>
>> Please help to answer above query as my patch is based on this pre-assumption
>> that one handler can support both callbacks .
>>
>> If we really need to have support of such handlers in input core then
>> only my patch
>> is good.
>
> I think I would need a user for this feature before changing the code to
> allow it.
>
> Thanks.
>
> --
> Dmitry

[Question : drivers/input ] Fixing Event Filter Mechanism in input subsystem

[Anshul Garg]

Dear Mr. Dmitry and Linux Community,

I am Anshul Garg working on Linux Kernel from last 2 years .

I have one query regarding event filter mechanism in input subsystem.
Can you please help in answering my query as below:

=====================================================
In function input_pass_values in input.c file , input core sends all
events to each handler associated with

the input device ,

rcu_read_lock();

handle = rcu_dereference(dev->grab);
if (handle) {
count = input_to_handler(handle, vals, count);
} else {
list_for_each_entry_rcu(handle, &dev->h_list, d_node)
if (handle->open)
count = input_to_handler(handle, vals, count);
}

after this in input_to_handler function events are filtered and sent
to the handler.

for (v = vals; v != vals + count; v++) {
if (handler->filter &&
   handler->filter(handle, v->type, v->code, v->value))
continue;
if (end != v)
*end = *v;
end++;
}



But as per previous event filter mechanism all the events should be
parsed from all
handlers after that remaining events should be sent to handlers list.

And in comments also its mentioned as

/*
 * Pass event first through all filters and then, if event has not been
 * filtered out, through all open handles.*/

So current approach to filter events seems to be incorrect.

Please help to clarify my query.


I have prepared one patch with new event filter mechanism which will
first filter all events from attached handlers then pass the remaining
events after being filtered
through all handlers unlike current implementation.

Once My query is resolved , I will send the patch.


Thanks
Anshul Garg
+91-9899777351