ext4: do not send discards as barriers

08-25-2010, 03:47 PM

From: Christoph Hellwig <hch@infradead.org>

ext4 already uses synchronous discards, no need to add I/O barriers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
fs/ext4/mballoc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index df44b34..a22bfef 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2567,7 +2567,7 @@ static inline void ext4_issue_discard(struct super_block *sb,
trace_ext4_discard_blocks(sb,
(unsigned long long) discard_block, count);
ret = sb_issue_discard(sb, discard_block, count, GFP_NOFS,
- BLKDEV_IFL_WAIT | BLKDEV_IFL_BARRIER);
+ BLKDEV_IFL_WAIT);
if (ret == EOPNOTSUPP) {
ext4_warning(sb, "discard not supported, disabling");
clear_opt(EXT4_SB(sb)->s_mount_opt, DISCARD);
--
1.7.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-25-2010, 03:57 PM

On 08/25/2010 06:00 PM, Christoph Hellwig wrote:
> On Wed, Aug 25, 2010 at 05:58:42PM +0200, Christoph Hellwig wrote:
>> On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
>>> From: Christoph Hellwig <hch@infradead.org>
>>>
>>> ext4 already uses synchronous discards, no need to add I/O barriers.
>>
>> This needs the patch that Jan sent in reply to my initial version merged
>> into it.
>
> Actually the jbd2 patch needs it merged, but the point still stands.

Yeah, wasn't sure about that one. Has anyone tested it? I'll be
happy to merge it but I have no idea whether it's correct or not and
Jan didn't seem to have tested it so... Jan, shall I merge the patch?

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-25-2010, 03:58 PM

On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
> From: Christoph Hellwig <hch@infradead.org>
>
> ext4 already uses synchronous discards, no need to add I/O barriers.

This needs the patch that Jan sent in reply to my initial version merged
into it.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-25-2010, 04:00 PM

On Wed, Aug 25, 2010 at 05:58:42PM +0200, Christoph Hellwig wrote:
> On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
> > From: Christoph Hellwig <hch@infradead.org>
> >
> > ext4 already uses synchronous discards, no need to add I/O barriers.
>
> This needs the patch that Jan sent in reply to my initial version merged
> into it.

Actually the jbd2 patch needs it merged, but the point still stands.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-25-2010, 08:02 PM

On Wed 25-08-10 17:57:41, Tejun Heo wrote:
> On 08/25/2010 06:00 PM, Christoph Hellwig wrote:
> > On Wed, Aug 25, 2010 at 05:58:42PM +0200, Christoph Hellwig wrote:
> >> On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
> >>> From: Christoph Hellwig <hch@infradead.org>
> >>>
> >>> ext4 already uses synchronous discards, no need to add I/O barriers.
> >>
> >> This needs the patch that Jan sent in reply to my initial version merged
> >> into it.
> >
> > Actually the jbd2 patch needs it merged, but the point still stands.
>
> Yeah, wasn't sure about that one. Has anyone tested it? I'll be
> happy to merge it but I have no idea whether it's correct or not and
> Jan didn't seem to have tested it so... Jan, shall I merge the patch?
I'm quite confident the patch is correct so you can merge it I think but
tomorrow I'll give it some crash testing together with the rest of your
patch set in KVM to be sure.

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-26-2010, 08:25 AM

On 08/25/2010 10:02 PM, Jan Kara wrote:
> On Wed 25-08-10 17:57:41, Tejun Heo wrote:
>> On 08/25/2010 06:00 PM, Christoph Hellwig wrote:
>>> On Wed, Aug 25, 2010 at 05:58:42PM +0200, Christoph Hellwig wrote:
>>>> On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
>>>>> From: Christoph Hellwig <hch@infradead.org>
>>>>>
>>>>> ext4 already uses synchronous discards, no need to add I/O barriers.
>>>>
>>>> This needs the patch that Jan sent in reply to my initial version merged
>>>> into it.
>>>
>>> Actually the jbd2 patch needs it merged, but the point still stands.
>>
>> Yeah, wasn't sure about that one. Has anyone tested it? I'll be
>> happy to merge it but I have no idea whether it's correct or not and
>> Jan didn't seem to have tested it so... Jan, shall I merge the patch?
> I'm quite confident the patch is correct so you can merge it I think but
> tomorrow I'll give it some crash testing together with the rest of your
> patch set in KVM to be sure.

Patch included in the series before jbd2 conversion patch.

Thanks.

--
tejun

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-27-2010, 05:31 PM

On Thu 26-08-10 10:25:47, Tejun Heo wrote:
> On 08/25/2010 10:02 PM, Jan Kara wrote:
> > On Wed 25-08-10 17:57:41, Tejun Heo wrote:
> >> On 08/25/2010 06:00 PM, Christoph Hellwig wrote:
> >>> On Wed, Aug 25, 2010 at 05:58:42PM +0200, Christoph Hellwig wrote:
> >>>> On Wed, Aug 25, 2010 at 05:47:43PM +0200, Tejun Heo wrote:
> >>>>> From: Christoph Hellwig <hch@infradead.org>
> >>>>>
> >>>>> ext4 already uses synchronous discards, no need to add I/O barriers.
> >>>>
> >>>> This needs the patch that Jan sent in reply to my initial version merged
> >>>> into it.
> >>>
> >>> Actually the jbd2 patch needs it merged, but the point still stands.
> >>
> >> Yeah, wasn't sure about that one. Has anyone tested it? I'll be
> >> happy to merge it but I have no idea whether it's correct or not and
> >> Jan didn't seem to have tested it so... Jan, shall I merge the patch?
> > I'm quite confident the patch is correct so you can merge it I think but
> > tomorrow I'll give it some crash testing together with the rest of your
> > patch set in KVM to be sure.
>
> Patch included in the series before jbd2 conversion patch.
An update: I've set up an ext4 barrier testing in KVM - run fsstress,
kill KVM at some random moment and check that the filesystem is consistent
(kvm is run in cache=writeback mode to simulate disk cache). About 70 runs
without journal_async_commit passed fine, now I'm running some tests with
the option enabled and the first few rounds passed OK as well.

Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-30-2010, 07:56 PM

Jan Kara <jack@suse.cz> writes:

> An update: I've set up an ext4 barrier testing in KVM - run fsstress,
> kill KVM at some random moment and check that the filesystem is consistent
> (kvm is run in cache=writeback mode to simulate disk cache). About 70 runs

But doesn't your "disk cache" survive the "power cycle" of your guest?
It's tough to tell exactly what you're testing with so few details;
care to elaborate?

Cheers,
Jeff

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-30-2010, 08:20 PM

On Mon 30-08-10 15:56:43, Jeff Moyer wrote:
> Jan Kara <jack@suse.cz> writes:
>
> > An update: I've set up an ext4 barrier testing in KVM - run fsstress,
> > kill KVM at some random moment and check that the filesystem is consistent
> > (kvm is run in cache=writeback mode to simulate disk cache). About 70 runs
>
> But doesn't your "disk cache" survive the "power cycle" of your guest?
Yes, you're right. Thinking about it now the test setup was wrong because
it didn't refuse writes to the VM's data partition after the moment I
killed KVM. Thanks for catching this. I will probably have to use the fault
injection on the host to disallow writing the device at a certain moment.
Or does somebody have a better option?
My setup is that I have a dedicated partition / drive for a filesystem
which is written to from a guest kernel running under KVM. I have set it up
using virtio driver with cache=writeback so that the host caches the writes
in a similar way disk caches them. At some point I just kill the qemu-kvm
process and at that point I'd like to also throw away data cached by the
host...

Honza

--
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

08-30-2010, 08:24 PM

On 08/30/2010 05:20 PM, Jan Kara wrote:

On Mon 30-08-10 15:56:43, Jeff Moyer wrote:

Jan Kara<jack@suse.cz> writes:

An update: I've set up an ext4 barrier testing in KVM - run fsstress,
kill KVM at some random moment and check that the filesystem is consistent
(kvm is run in cache=writeback mode to simulate disk cache). About 70 runs

But doesn't your "disk cache" survive the "power cycle" of your guest?

Yes, you're right. Thinking about it now the test setup was wrong because
it didn't refuse writes to the VM's data partition after the moment I
killed KVM. Thanks for catching this. I will probably have to use the fault
injection on the host to disallow writing the device at a certain moment.
Or does somebody have a better option?
My setup is that I have a dedicated partition / drive for a filesystem
which is written to from a guest kernel running under KVM. I have set it up
using virtio driver with cache=writeback so that the host caches the writes
in a similar way disk caches them. At some point I just kill the qemu-kvm
process and at that point I'd like to also throw away data cached by the
host...

Honza

Hi Jan,

Not sure if this is relevant, but what we have been using for part of the
testing is an external e-sata enclosure that you can stick pretty much any S-ATA
disk into. Important to drop power to the external disk (do not pull the s-ata
cable, the firmware will destage the write cache for some/many disks if it has
power and sees link loss

).

Once you turn the drive back on, the test was can you mount without error,
unmount and do a fsck -f to verify no meta-data corruption,

Ric

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel