Hello.
Back in August 2011, a commit has been tagged to be included
into stable, this one:
commit dccaf33fa37a1bc5d651baeb3bfeb6becb86597b
Author: Jiaying Zhang <[email protected]>
Date: Fri Aug 19 19:13:32 2011 -0400
ext4: flush any pending end_io requests before DIO reads w/dioread_nolock
There is a race between ext4 buffer write and direct_IO read with
dioread_nolock mount option enabled. The problem is that we clear
PageWriteback flag during end_io time but will do
uninitialized-to-initialized extent conversion later with dioread_nolock.
If an O_direct read request comes in during this period, ext4 will return
zero instead of the recently written data.
This patch checks whether there are any pending uninitialized-to-initialized
extent conversion requests before doing O_direct read to close the race.
Note that this is just a bandaid fix. The fundamental issue is that we
clear PageWriteback flag before we really complete an IO, which is
problem-prone. To fix the fundamental issue, we may need to implement an
extent tree cache that we can use to look up pending to-be-converted extents.
Signed-off-by: Jiaying Zhang <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
Cc: [email protected]
There was one more ext4 commit at that time, which made its way into
stable but this one did not.
I wonder if the reason for that was the fact that it needed a small
"backport" for 3.0, since in 3.1+ the code has been moved into another
file, and the context is slightly different. In that case, attached
is the "backport" which we use with 3.0.x since that time.
Thanks!
/mjt
Is there something wrong with my question? I asked it 1.5 months ago...
Meanwhile, we're using this patch on our database server since
Aug-2011, and it appears to work correctly - direct and buffered
I/O works together without surprizes. Without this patch, I see
unexpected results.
Thanks,
/mjt
On 01.12.2011 00:38, Michael Tokarev wrote:
> Hello.
>
> Back in August 2011, a commit has been tagged to be included
> into stable, this one:
>
> commit dccaf33fa37a1bc5d651baeb3bfeb6becb86597b
> Author: Jiaying Zhang <[email protected]>
> Date: Fri Aug 19 19:13:32 2011 -0400
>
> ext4: flush any pending end_io requests before DIO reads w/dioread_nolock
>
> There is a race between ext4 buffer write and direct_IO read with
> dioread_nolock mount option enabled. The problem is that we clear
> PageWriteback flag during end_io time but will do
> uninitialized-to-initialized extent conversion later with dioread_nolock.
> If an O_direct read request comes in during this period, ext4 will return
> zero instead of the recently written data.
>
> This patch checks whether there are any pending uninitialized-to-initialized
> extent conversion requests before doing O_direct read to close the race.
> Note that this is just a bandaid fix. The fundamental issue is that we
> clear PageWriteback flag before we really complete an IO, which is
> problem-prone. To fix the fundamental issue, we may need to implement an
> extent tree cache that we can use to look up pending to-be-converted extents.
>
> Signed-off-by: Jiaying Zhang <[email protected]>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
> Cc: [email protected]
>
>
> There was one more ext4 commit at that time, which made its way into
> stable but this one did not.
>
> I wonder if the reason for that was the fact that it needed a small
> "backport" for 3.0, since in 3.1+ the code has been moved into another
> file, and the context is slightly different. In that case, attached
> is the "backport" which we use with 3.0.x since that time.
>
> Thanks!
>
> /mjt
Ping?
Maybe just a one-line reply isn't THAT difficult?
We've a data corruption bug in current longterm stable kernel
series which is known and has a fix and tagged for -stable for
over half a year already...
Thanks,
/mjt
On 28.02.2012 15:42, Michael Tokarev wrote:
> Is there something wrong with my question? I asked it 1.5 months ago...
>
> Meanwhile, we're using this patch on our database server since
> Aug-2011, and it appears to work correctly - direct and buffered
> I/O works together without surprizes. Without this patch, I see
> unexpected results.
>
> Thanks,
>
> /mjt
>
> On 01.12.2011 00:38, Michael Tokarev wrote:
>> Hello.
>>
>> Back in August 2011, a commit has been tagged to be included
>> into stable, this one:
>>
>> commit dccaf33fa37a1bc5d651baeb3bfeb6becb86597b
>> Author: Jiaying Zhang <[email protected]>
>> Date: Fri Aug 19 19:13:32 2011 -0400
>>
>> ext4: flush any pending end_io requests before DIO reads w/dioread_nolock
>>
>> There is a race between ext4 buffer write and direct_IO read with
>> dioread_nolock mount option enabled. The problem is that we clear
>> PageWriteback flag during end_io time but will do
>> uninitialized-to-initialized extent conversion later with dioread_nolock.
>> If an O_direct read request comes in during this period, ext4 will return
>> zero instead of the recently written data.
>>
>> This patch checks whether there are any pending uninitialized-to-initialized
>> extent conversion requests before doing O_direct read to close the race.
>> Note that this is just a bandaid fix. The fundamental issue is that we
>> clear PageWriteback flag before we really complete an IO, which is
>> problem-prone. To fix the fundamental issue, we may need to implement an
>> extent tree cache that we can use to look up pending to-be-converted extents.
>>
>> Signed-off-by: Jiaying Zhang <[email protected]>
>> Signed-off-by: "Theodore Ts'o" <[email protected]>
>> Cc: [email protected]
>>
>>
>> There was one more ext4 commit at that time, which made its way into
>> stable but this one did not.
>>
>> I wonder if the reason for that was the fact that it needed a small
>> "backport" for 3.0, since in 3.1+ the code has been moved into another
>> file, and the context is slightly different. In that case, attached
>> is the "backport" which we use with 3.0.x since that time.
>>
>> Thanks!
>>
>> /mjt
>
On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
> Ping?
>
> Maybe just a one-line reply isn't THAT difficult?
>
> We've a data corruption bug in current longterm stable kernel
> series which is known and has a fix and tagged for -stable for
> over half a year already...
Greg, any idea why this patch was not included?
Honza
> On 28.02.2012 15:42, Michael Tokarev wrote:
> > Is there something wrong with my question? I asked it 1.5 months ago...
> >
> > Meanwhile, we're using this patch on our database server since
> > Aug-2011, and it appears to work correctly - direct and buffered
> > I/O works together without surprizes. Without this patch, I see
> > unexpected results.
> >
> > Thanks,
> >
> > /mjt
> >
> > On 01.12.2011 00:38, Michael Tokarev wrote:
> >> Hello.
> >>
> >> Back in August 2011, a commit has been tagged to be included
> >> into stable, this one:
> >>
> >> commit dccaf33fa37a1bc5d651baeb3bfeb6becb86597b
> >> Author: Jiaying Zhang <[email protected]>
> >> Date: Fri Aug 19 19:13:32 2011 -0400
> >>
> >> ext4: flush any pending end_io requests before DIO reads w/dioread_nolock
> >>
> >> There is a race between ext4 buffer write and direct_IO read with
> >> dioread_nolock mount option enabled. The problem is that we clear
> >> PageWriteback flag during end_io time but will do
> >> uninitialized-to-initialized extent conversion later with dioread_nolock.
> >> If an O_direct read request comes in during this period, ext4 will return
> >> zero instead of the recently written data.
> >>
> >> This patch checks whether there are any pending uninitialized-to-initialized
> >> extent conversion requests before doing O_direct read to close the race.
> >> Note that this is just a bandaid fix. The fundamental issue is that we
> >> clear PageWriteback flag before we really complete an IO, which is
> >> problem-prone. To fix the fundamental issue, we may need to implement an
> >> extent tree cache that we can use to look up pending to-be-converted extents.
> >>
> >> Signed-off-by: Jiaying Zhang <[email protected]>
> >> Signed-off-by: "Theodore Ts'o" <[email protected]>
> >> Cc: [email protected]
> >>
> >>
> >> There was one more ext4 commit at that time, which made its way into
> >> stable but this one did not.
> >>
> >> I wonder if the reason for that was the fact that it needed a small
> >> "backport" for 3.0, since in 3.1+ the code has been moved into another
> >> file, and the context is slightly different. In that case, attached
> >> is the "backport" which we use with 3.0.x since that time.
> >>
> >> Thanks!
> >>
> >> /mjt
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 19, 2012 at 9:42 AM, Jan Kara <[email protected]> wrote:
> On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
>> Ping?
>>
>> Maybe just a one-line reply isn't THAT difficult?
Sorry for the slow response. I am not sure what happened to this patch.
Ted, do you know what we need to do to get this patch to the
stable release?
Jiaying
>>
>> We've a data corruption bug in current longterm stable kernel
>> series which is known and has a fix and tagged for -stable for
>> over half a year already...
> ?Greg, any idea why this patch was not included?
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Honza
>
>> On 28.02.2012 15:42, Michael Tokarev wrote:
>> > Is there something wrong with my question? ?I asked it 1.5 months ago...
>> >
>> > Meanwhile, we're using this patch on our database server since
>> > Aug-2011, and it appears to work correctly - direct and buffered
>> > I/O works together without surprizes. ?Without this patch, I see
>> > unexpected results.
>> >
>> > Thanks,
>> >
>> > /mjt
>> >
>> > On 01.12.2011 00:38, Michael Tokarev wrote:
>> >> Hello.
>> >>
>> >> Back in August 2011, a commit has been tagged to be included
>> >> into stable, this one:
>> >>
>> >> commit dccaf33fa37a1bc5d651baeb3bfeb6becb86597b
>> >> Author: Jiaying Zhang <[email protected]>
>> >> Date: ? Fri Aug 19 19:13:32 2011 -0400
>> >>
>> >> ? ? ext4: flush any pending end_io requests before DIO reads w/dioread_nolock
>> >>
>> >> ? ? There is a race between ext4 buffer write and direct_IO read with
>> >> ? ? dioread_nolock mount option enabled. The problem is that we clear
>> >> ? ? PageWriteback flag during end_io time but will do
>> >> ? ? uninitialized-to-initialized extent conversion later with dioread_nolock.
>> >> ? ? If an O_direct read request comes in during this period, ext4 will return
>> >> ? ? zero instead of the recently written data.
>> >>
>> >> ? ? This patch checks whether there are any pending uninitialized-to-initialized
>> >> ? ? extent conversion requests before doing O_direct read to close the race.
>> >> ? ? Note that this is just a bandaid fix. The fundamental issue is that we
>> >> ? ? clear PageWriteback flag before we really complete an IO, which is
>> >> ? ? problem-prone. To fix the fundamental issue, we may need to implement an
>> >> ? ? extent tree cache that we can use to look up pending to-be-converted extents.
>> >>
>> >> ? ? Signed-off-by: Jiaying Zhang <[email protected]>
>> >> ? ? Signed-off-by: "Theodore Ts'o" <[email protected]>
>> >> ? ? Cc: [email protected]
>> >>
>> >>
>> >> There was one more ext4 commit at that time, which made its way into
>> >> stable but this one did not.
>> >>
>> >> I wonder if the reason for that was the fact that it needed a small
>> >> "backport" for 3.0, since in 3.1+ the code has been moved into another
>> >> file, and the context is slightly different. ?In that case, attached
>> >> is the "backport" which we use with 3.0.x since that time.
>> >>
>> >> Thanks!
>> >>
>> >> /mjt
>> >
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
On Mon, Mar 19, 2012 at 05:42:56PM +0100, Jan Kara wrote:
> On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
> > Ping?
> >
> > Maybe just a one-line reply isn't THAT difficult?
> >
> > We've a data corruption bug in current longterm stable kernel
> > series which is known and has a fix and tagged for -stable for
> > over half a year already...
> Greg, any idea why this patch was not included?
I have no idea, sorry, that was way back in August of 2011, I can barely
remember what patches were and were not applied last week...
If it's still needed for 3.0, let me know and I'll be glad to queue it
up.
thanks,
greg k-h
19.03.2012 21:10, Jiaying Zhang wrote:
> On Mon, Mar 19, 2012 at 9:42 AM, Jan Kara<[email protected]> wrote:
>> On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
>>> Ping?
>>>
>>> Maybe just a one-line reply isn't THAT difficult?
> Sorry for the slow response. I am not sure what happened to this patch.
> Ted, do you know what we need to do to get this patch to the
> stable release?
For reference, attached is the version I use locally since last August.
...
>>>>> There was one more ext4 commit at that time, which made its way into
>>>>> stable but this one did not.
>>>>>
>>>>> I wonder if the reason for that was the fact that it needed a small
>>>>> "backport" for 3.0, since in 3.1+ the code has been moved into another
>>>>> file, and the context is slightly different. In that case, attached
>>>>> is the "backport" which we use with 3.0.x since that time.
Thanks,
/mjt
On Mon, Mar 19, 2012 at 10:21 AM, Greg KH <[email protected]> wrote:
> On Mon, Mar 19, 2012 at 05:42:56PM +0100, Jan Kara wrote:
>> On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
>> > Ping?
>> >
>> > Maybe just a one-line reply isn't THAT difficult?
>> >
>> > We've a data corruption bug in current longterm stable kernel
>> > series which is known and has a fix and tagged for -stable for
>> > over half a year already...
>> ? Greg, any idea why this patch was not included?
>
> I have no idea, sorry, that was way back in August of 2011, I can barely
> remember what patches were and were not applied last week...
>
> If it's still needed for 3.0, let me know and I'll be glad to queue it
> up.
Yes. it is still needed. The commit ID of the change is
dccaf33fa37a1bc5d651baeb3bfeb6becb86597b.
Please let me know if there is any problem on applying the change. Thanks!
Jiaying
>
> thanks,
>
> greg k-h
On Mon, Mar 19, 2012 at 09:21:51PM +0400, Michael Tokarev wrote:
> 19.03.2012 21:10, Jiaying Zhang wrote:
> >On Mon, Mar 19, 2012 at 9:42 AM, Jan Kara<[email protected]> wrote:
> >>On Sat 17-03-12 13:31:30, Michael Tokarev wrote:
> >>>Ping?
> >>>
> >>>Maybe just a one-line reply isn't THAT difficult?
> >Sorry for the slow response. I am not sure what happened to this patch.
> >Ted, do you know what we need to do to get this patch to the
> >stable release?
>
> For reference, attached is the version I use locally since last August.
Wonderful, thanks for this, now queued up.
greg k-h