Re: [PATCH] fs: fput() can be called from interrupt context

[Eric Dumazet <dada1@cosmosbay.com>]

Andrew Morton a écrit :
> On Thu, 12 Mar 2009 06:18:26 +0100 Eric Dumazet <dada1@cosmosbay.com> wrote:
> 
>> Eric Dumazet wrote :
>>> Path could be :
>>>
>>> 1) fput() changes so that calling it from interrupt context is possible
>>>    (Using a working queue to make sure __fput() is called from process context)
>>>
>>> 2) Changes aio to use fput() as is (and zap its internal work_queue and aio_fput_routine() stuff)
>>>
>>> 3) Once atomic_long_dec_and_test(&filp->f_count) only performed in fput(),
>>>    SLAB_DESTROY_BY_RCU for "struct file" get back :)
>>>
>> Please find first patch against linux-2.6
>>
>> Next patch (2) can cleanup aio code, but it probably can wait linux-2.6.30
>>
>> Thank you
>>
>> [PATCH] fs: fput() can be called from interrupt context
>>
>> Current aio/eventfd code can call fput() from interrupt context, which is
>> not allowed.
> 
> The changelog forgot to tell us where this happens, and under what
> circumstances.
> 
> See, there might be other ways of fixing the bug,

Sure

> 
>> In order to fix the problem and prepare SLAB_DESTROY_BY_RCU use for "struct file"
>> allocation/freeing in 2.6.30, we might extend existing workqueue infrastructure and
>> allow fput() to be called from interrupt context.
>>
>> This unfortunalty adds a pointer to 'struct file'.
>>
>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
>> ---
>>  fs/file.c               |   55 ++++++++++++++++++++++++++------------
>>  fs/file_table.c         |   10 +++++-
>>  include/linux/fdtable.h |    1
>>  include/linux/fs.h      |    1
>>  4 files changed, 49 insertions(+), 18 deletions(-)
> 
> which might not have some or all of the above problems.
> 
> 
> I assume you're referring to really_put_req(), and commit
> 9c3060bedd84144653a2ad7bea32389f65598d40.
> 
>>From the above email straggle I extract "If user program closes
> eventfd, then inflight AIO requests can trigger a bug" and I don't
> immediately see anything in there which would prevent this.
> 
> Did you reproduce the bug, and confirm that the patch fixes it?

take Davide program : http://www.xmailserver.org/eventfd-aio-test.c

and add at line 318 :
close(afd);

It should produce the kernel bug...
> 
> Are there simpler ways of fixing it?  Maybe sneak a call to
> wait_for_all_aios() into the right place?  I doubt if it's performance
> critical, as nobody seems to have ever hit the bug.

Take the time to check how fs/aio.c handle the fput(req->ki_filp) case
(or read my 2nd patch, it should spot the thing)

If you want to add another kludge to properly fput(req->ki_eventfd),
be my guest :-(

> 
> Bear in mind that if the bug _is_ real then it's now out there, and
> we would like a fix which is usable by 2.6.<two-years-worth>.
> 
> etcetera..