2006-05-05 02:19:55

by Constantine Sapuntzakis

[permalink] [raw]
Subject: [PATCH] loop.c: respect bio barrier and sync

I believe that the loop block device does not currently respect
barriers or syncs issued by its clients. As a result, I have seen
corrupted log errors when a loopback mounted ext3 file system is
remounted after a hard stop.

The attached patch attempts to fix this problem by respecting the
barrier and sync flags on the I/O request. The sync_file function was
cut-and-paste from the implementation of fsync (I think there's no fd
so I can't call fsync) to allow the patch to be deployed as an updated
module. Is there another function that could be used?

Comments are welcome. I am not on the list so please cc: me on any response.

-Costa


Attachments:
(No filename) (642.00 B)
loop-barrier-patch.diff (1.40 kB)
Download all attachments

2006-05-05 14:54:41

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] loop.c: respect bio barrier and sync

On Thu, May 04 2006, Constantine Sapuntzakis wrote:
> I believe that the loop block device does not currently respect
> barriers or syncs issued by its clients. As a result, I have seen
> corrupted log errors when a loopback mounted ext3 file system is
> remounted after a hard stop.
>
> The attached patch attempts to fix this problem by respecting the
> barrier and sync flags on the I/O request. The sync_file function was
> cut-and-paste from the implementation of fsync (I think there's no fd
> so I can't call fsync) to allow the patch to be deployed as an updated
> module. Is there another function that could be used?
>
> Comments are welcome. I am not on the list so please cc: me on any
> response..

Please inline your patches, so one can actually comment on them...

- You should handle sync_file() failure. If we don't have !f_op (will
that ever hit, btw?) or ->fsync(), then fail the barrier with
-EOPNOTSUPP. For fsync failure, well... You probably want to just
error the bio with -EIO then.

- bio_sync() doesn't have the semantics you define it to, it is a hint
to the block layer to start request processing instead of plugging. So
don't treat it as a barrier, ignore it.

- Does this work for all loop_device types?

--
Jens Axboe

2006-05-05 22:37:11

by Constantine Sapuntzakis

[permalink] [raw]
Subject: Re: [PATCH] loop.c: respect bio barrier and sync

Sorry about the patch as attachments.

> - You should handle sync_file() failure. If we don't have !f_op (will
> that ever hit, btw?) or ->fsync(), then fail the barrier with
> -EOPNOTSUPP. For fsync failure, well... You probably want to just
> error the bio with -EIO then.

Will fix.

> - Does this work for all loop_device types?

What are the other loop_device types?

BTW, should/does an fsync on a block device translate into a disk flush?
I was looking at sync_blockdev and couldn't figure out how that happened.

Thanks,
-Costa