LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Ananiev, Leonid I" <leonid.i.ananiev@intel.com>
To: "Zach Brown" <zach.brown@oracle.com>
Cc: "Ken Chen" <kenchen@google.com>, <suparna@in.ibm.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	<linux-kernel@vger.kernel.org>, "linux-aio" <linux-aio@kvack.org>,
	"Chris Mason" <chris.mason@oracle.com>
Subject: RE: [PATCH] aio: fix kernel bug when page is temporally busy
Date: Fri, 16 Feb 2007 02:32:11 +0300	[thread overview]
Message-ID: <B41635854730A14CA71C92B36EC22AAC83AA05@mssmsx411> (raw)
In-Reply-To: <7B7DC266-39EB-40AB-B797-A92329120FEE@oracle.com>

>> If EIOCBRETRY then generic_file_aio_write() will be recalled for the
>> same iocb.
> Only if kick_iocb() is called.  It won't be called if i_i_p2_r() was  
> the only thing to return -EIOCBRETRY.
It is not need to call kick_iocb()
for generic_file_aio_write() calling.
It is recalled without any wakeup waiting:
        for (;;) {
                ret = filp->f_op->aio_write(&kiocb, &iov, 1,
kiocb.ki_pos);
                if (ret != -EIOCBRETRY)
                        break;
                wait_on_retry_sync_kiocb(&kiocb);
        }
Note: wait_on_retry_sync_kiocb() does not wait.
That is for dio. For aio iocb generic_file_aio_write() call
is required from ki_run_list while next io_submit or
read_events() is called.
So when an IO hang may happen?

>> It overwrites -EIOCBQUEUED
Do you mean that there is one more kernel bug which
overwrites -EIOCBQUEUED by any errno or number of bytes and this
new value is returned to caller as an IO result
while IO is not finished yet.

The proposed patch does not crate this bug if any.
It actually fixes a kernel panic bag when iocb.users count becomes
incorrect. The bag " Kernel BUG at fs/aio.c:509" is there because
aio_run_iocb() have not a chance to differ real EIO and
EIO which is actually means EAGAYN or EIOCBRETRY.
I'me sure the patch changes source code in correct direction:
to differentiate that two kinds of EIOs.

> Have you read the giant comment over the definition of struct kiocb  
> in include/linux/aio.h?
I have read. But compiler has not: it did not create an object code for
* If ki_retry returns -EIOCBRETRY ...

>>> This can lead to reference count confusion.
>> But just reference count confusion was deleted by patch. Isn't it?
>Sorry, I don't understand what you're trying to ask here.
One of reference count iocb.users confusion is deleted by the patch.
I'm not sure that there is other bag.

At least I have not see IO hang while testing.
It is interesting that I've not seen any EIOCBQUEUED returned
to aio_run_iocb() during 5 hours aiostress running.
Does it mean that EIOCBQUEUED is always reset and never returned?

Leonid

-----Original Message-----
From: Zach Brown [mailto:zach.brown@oracle.com] 
Sent: Thursday, February 15, 2007 10:23 PM
To: Ananiev, Leonid I
Cc: Ken Chen; suparna@in.ibm.com; Andrew Morton;
linux-kernel@vger.kernel.org; linux-aio; Chris Mason
Subject: Re: [PATCH] aio: fix kernel bug when page is temporally busy


On Feb 15, 2007, at 11:11 AM, Ananiev, Leonid I wrote:

>> It returns -EIOCBRETRY without guaranteeing that kick_iocb() will be
>> called.  This can lead to operations hanging
>
> If EIOCBRETRY then generic_file_aio_write() will be recalled for the
> same iocb.

Only if kick_iocb() is called.  It won't be called if i_i_p2_r() was  
the only thing to return -EIOCBRETRY.

>
>> It overwrites -EIOCBQUEUED, leading to an aio_complete() while a
>> retry is happening.
>
> EIOCBQUEUED or EIOCBRETRY does not lead to aio_complete() call:

Not by fs/aio.c, but *by the place that originated -EIOCBQUEUED*.   
Later.  After IO has completed.  see fs/direct-io.c:dio_bio_end_aio().

This is what -EIOCBQUEUED means!  It's a promise to call aio_complete 
() in the future.

Have you read the giant comment over the definition of struct kiocb  
in include/linux/aio.h?

>> This can lead to reference count confusion.
> But just reference count confusion was deleted by patch. Isn't it?

Sorry, I don't understand what you're trying to ask here.

- z

  parent reply	other threads:[~2007-02-15 23:32 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-09  4:29 Ananiev, Leonid I
2007-02-09  4:35 ` Andrew Morton
2007-02-09  5:41   ` Ananiev, Leonid I
2007-02-09  5:52     ` Andrew Morton
2007-02-12 22:52       ` Ananiev, Leonid I
2007-02-12 23:21       ` Ananiev, Leonid I
2007-02-09  7:16     ` Suparna Bhattacharya
2007-02-09  9:52       ` Ananiev, Leonid I
2007-02-09 10:11         ` Jiri Kosina
2007-02-10 18:05         ` Ken Chen
2007-02-10 18:17           ` Ananiev, Leonid I
2007-02-10 18:27           ` Ananiev, Leonid I
2007-02-10 21:57           ` Ananiev, Leonid I
2007-02-15  9:16           ` Ananiev, Leonid I
2007-02-15 18:25             ` Zach Brown
2007-02-15 19:11               ` Ananiev, Leonid I
2007-02-15 19:22                 ` Zach Brown
2007-02-15 21:06                   ` Ananiev, Leonid I
2007-02-15 23:32                   ` Ananiev, Leonid I [this message]
2007-02-16  0:01                     ` Zach Brown
2007-02-16 12:18                       ` Ananiev, Leonid I
2007-02-09  9:54 ` Jiri Kosina
2007-02-09 10:14   ` Andrew Morton
2007-02-09 10:40     ` Jiri Kosina
2007-02-09 11:05       ` Suparna Bhattacharya
2007-02-09 11:18         ` Ananiev, Leonid I
2007-02-09 17:02         ` Zach Brown
2007-02-10 19:36 Ananiev, Leonid I
2007-02-14 17:51 Ananiev, Leonid I
2007-02-15  3:30 ` Andrew Morton
2007-02-15  5:26   ` Ananiev, Leonid I

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B41635854730A14CA71C92B36EC22AAC83AA05@mssmsx411 \
    --to=leonid.i.ananiev@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=chris.mason@oracle.com \
    --cc=kenchen@google.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suparna@in.ibm.com \
    --cc=zach.brown@oracle.com \
    --subject='RE: [PATCH] aio: fix kernel bug when page is temporally busy' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).