f2fs: remove broken support for allocating DIO writes

From: Eric Biggers <[email protected]>

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete. This can be easily reproduced by issuing a DIO write that will fail due to misalignment, e.g.:

rm -f file
truncate -s 1000000 file
dd if=/dev/zero bs=999999 oflag=direct conv=notrunc of=file od -tx1 file # shows uninitialized disk blocks

Until a proper design for non-overwrite DIO writes on f2fs can be designed and implemented, remove support for them and make them fall back to buffered I/O. This is what other filesystems that don't support unwritten blocks, e.g. ext2, also do, at least for non-extending DIO writes. However, f2fs can't do extending DIO writes either, as f2fs appears to have no mechanism for guaranteeing that leftover allocated blocks past EOF will get truncated. (f2fs does have an orphan list, but it's only used for deleting inodes, not truncating them.)

This patch doesn't attempt to remove the F2FS_GET_BLOCK_{DIO,PRE_DIO} cases in f2fs_map_blocks(); that can be cleaned up later.

Fixes bfad7c2d4033 ("f2fs: introduce a new direct_IO write path")

Cc [email protected]

Signed-off-by Eric Biggers <[email protected]>

---

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a

Quoted 3 lines

result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete. This can be easily reproduced by issuing a DIO write that will fail due to misalignment, e.g.:

Quoted 15 lines

rm -f file
truncate -s 1000000 file
dd if=/dev/zero bs=999999 oflag=direct conv=notrunc of=file od -tx1 file # shows uninitialized disk blocks

Until a proper design for non-overwrite DIO writes on f2fs can be designed and implemented, remove support for them and make them fall back to buffered I/O. This is what other filesystems that don't support unwritten blocks, e.g. ext2, also do, at least for non-extending DIO writes. However, f2fs can't do extending DIO writes either, as f2fs appears to have no mechanism for guaranteeing that leftover allocated blocks past EOF will get truncated. (f2fs does have an orphan list, but it's only used for deleting inodes, not truncating them.)

Quoted 7 lines

This patch doesn't attempt to remove the F2FS_GET_BLOCK_{DIO,PRE_DIO} cases in f2fs_map_blocks(); that can be cleaned up later.

Fixes bfad7c2d4033 ("f2fs: introduce a new direct_IO write path")

Cc [email protected]

Signed-off-by Eric Biggers <[email protected]>

---

Any opinion on this patch? This really needs to be fixed one way or another. Probably before the conversion to iomap, as this fix will need to be backported.

- Eric

Quoted 7 lines · Eric Biggers

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a

Hmm, I'm still wondering why this becomes a problem. And, do we really need to roll back the preallocated blocks?

Quoted 3 lines

Quoted 3 lines

result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete. This can be easily reproduced by issuing a DIO write that will fail due to misalignment, e.g.:

If there's any error, truncating blocks having NEW_ADDR could address this?

Quoted 16 lines · Jaegeuk Kim

Quoted 7 lines · Eric Biggers

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a

Hmm, I'm still wondering why this becomes a problem. And, do we really need to roll back the preallocated blocks?

Quoted 3 lines

Quoted 3 lines

result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete. This can be easily reproduced by issuing a DIO write that will fail due to misalignment, e.g.:

Quoted 2 lines

If there's any error, truncating blocks having NEW_ADDR could address this?

My understanding is that the "NEW_ADDR" block address in f2fs means that space was reserved for the block, but not allocated in any particular place yet. Buffered writes reserve blocks in this way, but DIO writes cannot because DIO by definition has to directly write to a specific on-disk location. Therefore DIO writes require that the blocks be preallocated for real.

- Eric

Quoted 17 lines · Eric Biggers

Quoted 16 lines · Jaegeuk Kim

Quoted 7 lines · Eric Biggers

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a

Hmm, I'm still wondering why this becomes a problem. And, do we really need to roll back the preallocated blocks?

Quoted 3 lines

Quoted 3 lines

result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete. This can be easily reproduced by issuing a DIO write that will fail due to misalignment, e.g.:

Quoted 8 lines

Quoted 2 lines

If there's any error, truncating blocks having NEW_ADDR could address this?

My understanding is that the "NEW_ADDR" block address in f2fs means that space was reserved for the block, but not allocated in any particular place yet. Buffered writes reserve blocks in this way, but DIO writes cannot because DIO by definition has to directly write to a specific on-disk location. Therefore DIO writes require that the blocks be preallocated for real.

Sorry, checking back the DIO flow, we do allocate real block addresses if DIO has holes.

f2fs_preallocate_blocks
-> f2fs_map_blocks(F2FS_GET_BLOCK_PRE_DIO)
-> __allocate_data_block()
-> f2fs_allocate_data_block() gets a free LBA

Then, back to your concern, do we need to truncate blocks beyond i_size, if we meet any failure?

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Cheers,

- Ted

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

- Eric

Quoted 18 lines · Eric Biggers

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Thanks for your suggestion, I think it makes sense.

Quoted 6 lines

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

Firstly, let's notice that below metadata will be touched during DIO preallocation flow:
- log header
- sit bitmap/count
- free seg/sec bitmap/count
- dirty seg/sec bitmap/count

And there is one case we need to concern about is: checkpoint() can be triggered randomly in between dio_preallocate() and dio_end_io(), we should not persist any DIO preallocation related metadata during checkpoint(), otherwise, sudden power-cut after the checkpoint will corrupt filesytem.

So it needs to well separate two kinds of metadata update:
a) belong to dio preallocation
b) the left one

After that, it will simply checkpoint() flow to just flush metadata b), for other flow, like GC, data/node allocation, it needs to query/update metadata after we combine metadata a) and b).

In addition, there is an existing in-memory log header framework in f2fs, based on this fwk, it's very easy for us to add a new in-memory log header for DIO preallocation.

So it seems feasible for me until now...

Jaegeuk, any other concerns about the implementation details?

Thanks,

Quoted 3 lines

- Eric

Quoted 56 lines · Chao Yu

Quoted 18 lines · Eric Biggers

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Thanks for your suggestion, I think it makes sense.

Quoted 6 lines

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

Firstly, let's notice that below metadata will be touched during DIO preallocation flow:
- log header
- sit bitmap/count
- free seg/sec bitmap/count
- dirty seg/sec bitmap/count

And there is one case we need to concern about is: checkpoint() can be triggered randomly in between dio_preallocate() and dio_end_io(), we should not persist any DIO preallocation related metadata during checkpoint(), otherwise, sudden power-cut after the checkpoint will corrupt filesytem.

So it needs to well separate two kinds of metadata update:
a) belong to dio preallocation
b) the left one

After that, it will simply checkpoint() flow to just flush metadata b), for other flow, like GC, data/node allocation, it needs to query/update metadata after we combine metadata a) and b).

In addition, there is an existing in-memory log header framework in f2fs, based on this fwk, it's very easy for us to add a new in-memory log header for DIO preallocation.

Quoted 3 lines

So it seems feasible for me until now...

Jaegeuk, any other concerns about the implementation details?

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

Quoted 7 lines

Thanks,

Quoted 3 lines

- Eric

Quoted 59 lines · Jaegeuk Kim

Quoted 58 lines · Chao Yu

Quoted 18 lines · Eric Biggers

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Thanks for your suggestion, I think it makes sense.

Quoted 6 lines

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

Firstly, let's notice that below metadata will be touched during DIO preallocation flow:
- log header
- sit bitmap/count
- free seg/sec bitmap/count
- dirty seg/sec bitmap/count

And there is one case we need to concern about is: checkpoint() can be triggered randomly in between dio_preallocate() and dio_end_io(), we should not persist any DIO preallocation related metadata during checkpoint(), otherwise, sudden power-cut after the checkpoint will corrupt filesytem.

So it needs to well separate two kinds of metadata update:
a) belong to dio preallocation
b) the left one

After that, it will simply checkpoint() flow to just flush metadata b), for other flow, like GC, data/node allocation, it needs to query/update metadata after we combine metadata a) and b).

In addition, there is an existing in-memory log header framework in f2fs, based on this fwk, it's very easy for us to add a new in-memory log header for DIO preallocation.

So it seems feasible for me until now...

Quoted 5 lines

Quoted 1 line

Jaegeuk, any other concerns about the implementation details?

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

What about the case that CP + SPO following DIO preallocation? User will encounter uninitialized block after recovery.

Thanks,

Quoted 7 lines

Quoted 7 lines

Thanks,

Quoted 3 lines

- Eric

Quoted 66 lines · Chao Yu

Quoted 64 lines · Jaegeuk Kim

Quoted 52 lines · Chao Yu

Quoted 18 lines · Eric Biggers

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Thanks for your suggestion, I think it makes sense.

Quoted 6 lines

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

Firstly, let's notice that below metadata will be touched during DIO preallocation flow:
- log header
- sit bitmap/count
- free seg/sec bitmap/count
- dirty seg/sec bitmap/count

And there is one case we need to concern about is: checkpoint() can be triggered randomly in between dio_preallocate() and dio_end_io(), we should not persist any DIO preallocation related metadata during checkpoint(), otherwise, sudden power-cut after the checkpoint will corrupt filesytem.

So it needs to well separate two kinds of metadata update:
a) belong to dio preallocation
b) the left one

After that, it will simply checkpoint() flow to just flush metadata b), for other flow, like GC, data/node allocation, it needs to query/update metadata after we combine metadata a) and b).

Quoted 7 lines

In addition, there is an existing in-memory log header framework in f2fs, based on this fwk, it's very easy for us to add a new in-memory log header for DIO preallocation.

So it seems feasible for me until now...

Jaegeuk, any other concerns about the implementation details?

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

Quoted 2 lines

What about the case that CP + SPO following DIO preallocation? User will encounter uninitialized block after recovery.

I think buffered writes as a workaround can expose the last unwritten block as well, if SPO happens right after block allocation. We may need to compromise at certain level?

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Quoted 8 lines · Eric Biggers

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

Quoted 5 lines

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Btw, this is generally a problem for buffered I/O as well, although the window for exposing uninitialized blocks on a crash tends to be smaller.

Quoted 15 lines · Christoph Hellwig

Quoted 13 lines · Eric Biggers

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Quoted 2 lines

Btw, this is generally a problem for buffered I/O as well, although the window for exposing uninitialized blocks on a crash tends to be smaller.

How about adding a warning message when we meet an error with preallocated unwritten blocks? In the meantime, can we get the Eric's patches for iomap support? I feel that we only need to modify the preallocation and error handling parts?

Quoted 19 lines · Jaegeuk Kim

Quoted 17 lines · Christoph Hellwig

Quoted 8 lines · Eric Biggers

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

Quoted 5 lines

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Btw, this is generally a problem for buffered I/O as well, although the window for exposing uninitialized blocks on a crash tends to be smaller.

Quoted 4 lines

How about adding a warning message when we meet an error with preallocated unwritten blocks? In the meantime, can we get the Eric's patches for iomap support? I feel that we only need to modify the preallocation and error handling parts?

A warning message would do nothing to prevent uninitialized blocks from being leaked to userspace.

- Eric

Quoted 25 lines · Eric Biggers

Quoted 23 lines · Jaegeuk Kim

Quoted 9 lines · Christoph Hellwig

Quoted 8 lines · Eric Biggers

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

Quoted 8 lines

Quoted 5 lines

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Btw, this is generally a problem for buffered I/O as well, although the window for exposing uninitialized blocks on a crash tends to be smaller.

How about adding a warning message when we meet an error with preallocated unwritten blocks? In the meantime, can we get the Eric's patches for iomap support? I feel that we only need to modify the preallocation and error handling parts?

Quoted 2 lines

A warning message would do nothing to prevent uninitialized blocks from being leaked to userspace.

To give a signal that it's a known issue that we'll fix later.

Quoted 67 lines · Jaegeuk Kim

Quoted 66 lines · Chao Yu

Quoted 64 lines · Jaegeuk Kim

Quoted 52 lines · Chao Yu

Quoted 18 lines · Eric Biggers

Quoted 17 lines · Theodore Ts'o

Quoted 6 lines · Eric Biggers

Quoted 6 lines

Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as they require preallocating blocks, but f2fs doesn't support unwritten blocks and therefore has to preallocate the blocks as regular blocks. f2fs has no way to reliably roll back such preallocations, so as a result, f2fs will leak uninitialized blocks to users if a DIO write doesn't fully complete.

There's another way of solving this problem which doesn't require supporting unwritten blocks. What a file system *could* do is to allocate the blocks, but *not* update the on-disk data structures --- so the allocation happens in memory only, so you know that the physical blocks won't get used for another files, and then issue the data block writes. On the block I/O completion, trigger a workqueue function which updates the on-disk metadata to assign physical blocks to the inode.

Quoted 3 lines

Quoted 3 lines

That way if you crash before the data I/O has a chance to complete, the on-disk logical block -> physical block map hasn't been updated yet, and so you don't need to worry about leaking uninitialized blocks.

Thanks for your suggestion, I think it makes sense.

Quoted 6 lines

Quoted 4 lines

Cheers,

- Ted

Jaegeuk and Chao, any idea how feasible it would be for f2fs to do this?

Firstly, let's notice that below metadata will be touched during DIO preallocation flow:
- log header
- sit bitmap/count
- free seg/sec bitmap/count
- dirty seg/sec bitmap/count

And there is one case we need to concern about is: checkpoint() can be triggered randomly in between dio_preallocate() and dio_end_io(), we should not persist any DIO preallocation related metadata during checkpoint(), otherwise, sudden power-cut after the checkpoint will corrupt filesytem.

So it needs to well separate two kinds of metadata update:
a) belong to dio preallocation
b) the left one

After that, it will simply checkpoint() flow to just flush metadata b), for other flow, like GC, data/node allocation, it needs to query/update metadata after we combine metadata a) and b).

Quoted 7 lines

In addition, there is an existing in-memory log header framework in f2fs, based on this fwk, it's very easy for us to add a new in-memory log header for DIO preallocation.

So it seems feasible for me until now...

Jaegeuk, any other concerns about the implementation details?

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

Quoted 7 lines

Quoted 2 lines

What about the case that CP + SPO following DIO preallocation? User will encounter uninitialized block after recovery.

I think buffered writes as a workaround can expose the last unwritten block as well, if SPO happens right after block allocation. We may need to compromise at certain level?

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

- Eric

Quoted 21 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

Quoted 7 lines

Quoted 2 lines

What about the case that CP + SPO following DIO preallocation? User will encounter uninitialized block after recovery.

I think buffered writes as a workaround can expose the last unwritten block as well, if SPO happens right after block allocation. We may need to compromise at certain level?

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

Quoted 5 lines

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

I may have time to take look into the implementation as I proposed above, maybe just enabling this in FSYNC_MODE_STRICT mode if user concerns unwritten data? thoughts?

Quoted 27 lines · Chao Yu

Quoted 26 lines

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Hmm, I'm still trying to deal with this as a corner case where the writes haven't completed due to an error. How about keeping the preallocated block offsets and releasing them if we get an error? Do we need to handle EIO right?

Quoted 7 lines

Quoted 2 lines

What about the case that CP + SPO following DIO preallocation? User will encounter uninitialized block after recovery.

I think buffered writes as a workaround can expose the last unwritten block as well, if SPO happens right after block allocation. We may need to compromise at certain level?

Freeing preallocated blocks on error would be better than nothing, although note that the preallocated blocks may have filled an arbitrary sequence of holes -- so simply truncating past EOF would *not* be sufficient.

But really filesystems need to be designed to never expose uninitialized data, even if I/O errors or a sudden power failure occurs. It is unfortunate that f2fs apparently wasn't designed with that goal in mind.

In any case, I don't think we can proceed with any other f2fs direct I/O improvements until this data leakage bug can be solved one way or another. If my patch to remove support for allocating writes isn't acceptable and the desired solution is going to require some more invasive f2fs surgery, are you or Chao going to work on it? I'm not sure there's much I can do here.

Quoted 4 lines

I may have time to take look into the implementation as I proposed above, maybe just enabling this in FSYNC_MODE_STRICT mode if user concerns unwritten data? thoughts?

What does this have to do with fsync?

- Eric