Skip to content

Commit 51c6935

Browse files
committed
Fix lots of bugs due to newer kernel 5.8 in Ubuntu 20.04, and OpenZFS v0.8:
1. clone_extents_to() didn't handle partial clones, and now it does. 2. OpenZFS has a bug in SEEK_DATA which caused empty clones, we use a hacky workaround to fix it. 3. mapped_file_handle now fails loudly if it fails to allocate the map due to VMA exhaustion. This closes off a ton of unpleasant surprise in heavily loaded applications. 4. If your Linux kernel was old enough, or you are on ZFS, renaming a mapped_file_handle caused internal state corruption which led to lots of bad things, including `bad_file_descriptor` errors. 5. On Windows, renaming a mapped_file_handle now tears down and restores the maps, as you cannot rename a file with open maps.
1 parent a74411e commit 51c6935

File tree

7 files changed

+276
-77
lines changed

7 files changed

+276
-77
lines changed

include/llfio/revision.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
// Note the second line of this file must ALWAYS be the git SHA, third line ALWAYS the git SHA update time
2-
#define LLFIO_PREVIOUS_COMMIT_REF 74e710643944764a3548847322d6cb9a4a7ed5ab
3-
#define LLFIO_PREVIOUS_COMMIT_DATE "2021-02-23 11:27:11 +00:00"
4-
#define LLFIO_PREVIOUS_COMMIT_UNIQUE 74e71064
2+
#define LLFIO_PREVIOUS_COMMIT_REF a74411eddb6401ab884c5f92cccc24b9a64a9e6f
3+
#define LLFIO_PREVIOUS_COMMIT_DATE "2021-02-23 14:33:57 +00:00"
4+
#define LLFIO_PREVIOUS_COMMIT_UNIQUE a74411ed

include/llfio/v2.0/detail/impl/posix/file_handle.ipp

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,18 @@ result<file_handle::extent_pair> file_handle::clone_extents_to(file_handle::exte
581581
for(;;)
582582
{
583583
#ifdef __linux__
584+
if(1)
585+
{
586+
/* There is a bug in ZFSOnLinux where SEEK_DATA returns no data in file
587+
if the file was rewritten using a mmap which is then closed, and the file
588+
immediately reopened and SEEK_DATA called upon it. If you read a single
589+
byte, it seems to kick SEEK_DATA into working correctly.
590+
591+
See https://github.com/openzfs/zfs/issues/11697 for more.
592+
*/
593+
char b;
594+
(void) ::pread(_v.fd, &b, 1, end);
595+
}
584596
start = lseek64(_v.fd, end, SEEK_DATA);
585597
#else
586598
start = lseek(_v.fd, end, SEEK_DATA);
@@ -747,17 +759,17 @@ result<file_handle::extent_pair> file_handle::clone_extents_to(file_handle::exte
747759
auto _copy_file_range = [&](int infd, off_t *inoffp, int outfd, off_t *outoffp, size_t len, unsigned int flags) -> ssize_t {
748760
#if defined(__linux__)
749761
#if defined __aarch64__
750-
return syscall(285 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
762+
return syscall(285 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
751763
#elif defined __arm__
752-
return syscall(391 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
764+
return syscall(391 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
753765
#elif defined __i386__
754-
return syscall(377 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
766+
return syscall(377 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
755767
#elif defined __powerpc64__
756-
return syscall(379 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
768+
return syscall(379 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
757769
#elif defined __sparc__
758-
return syscall(357 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
770+
return syscall(357 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
759771
#elif defined __x86_64__
760-
return syscall(326 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
772+
return syscall(326 /*__NR_copy_file_range*/, infd, inoffp, outfd, outoffp, len, flags);
761773
#else
762774
#error Unknown Linux platform
763775
#endif
@@ -790,23 +802,30 @@ result<file_handle::extent_pair> file_handle::clone_extents_to(file_handle::exte
790802
{
791803
for(extent_type thisoffset = 0; thisoffset < item.src.length; thisoffset += blocksize)
792804
{
805+
retry_clone:
793806
bool done = false;
794807
const auto thisblock = std::min(blocksize, item.src.length - thisoffset);
795808
if(duplicate_extents && item.op == workitem::clone_extents)
796809
{
797810
off_t off_in = item.src.offset + thisoffset, off_out = item.src.offset + thisoffset + destoffsetdiff;
798-
if(_copy_file_range(_v.fd, &off_in, dest.native_handle().fd, &off_out, thisblock, 0) < 0)
811+
auto bytes_cloned = _copy_file_range(_v.fd, &off_in, dest.native_handle().fd, &off_out, thisblock, 0);
812+
if(bytes_cloned <= 0)
799813
{
800-
if((EXDEV != errno && EOPNOTSUPP != errno && ENOSYS != errno) || !emulate_if_unsupported)
814+
if(bytes_cloned < 0 && ((EXDEV != errno && EOPNOTSUPP != errno && ENOSYS != errno) || !emulate_if_unsupported))
801815
{
802816
return posix_error();
803817
}
804818
duplicate_extents = false; // emulate using copy of bytes
805819
}
806-
else
820+
else if((size_t) bytes_cloned == thisblock)
807821
{
808822
done = true;
809823
}
824+
else
825+
{
826+
thisoffset += bytes_cloned;
827+
goto retry_clone;
828+
}
810829
}
811830
if(!done && (item.op == workitem::copy_bytes || (!duplicate_extents && item.op == workitem::clone_extents)))
812831
{

include/llfio/v2.0/detail/impl/posix/mapped_file_handle.ipp

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* An mapped handle to a file
2-
(C) 2017 Niall Douglas <http://www.nedproductions.biz/> (11 commits)
2+
(C) 2017-2021 Niall Douglas <http://www.nedproductions.biz/> (11 commits)
33
File Created: Sept 2017
44
55
@@ -27,15 +27,20 @@ Distributed under the Boost Software License, Version 1.0.
2727

2828
LLFIO_V2_NAMESPACE_BEGIN
2929

30-
result<mapped_file_handle::size_type> mapped_file_handle::reserve(size_type reservation) noexcept
30+
result<mapped_file_handle::size_type> mapped_file_handle::_reserve(extent_type &length, size_type reservation) noexcept
3131
{
3232
LLFIO_LOG_FUNCTION_CALL(this);
33-
OUTCOME_TRY(auto &&length, underlying_file_maximum_extent());
33+
#ifndef NDEBUG
34+
if(_mh.is_valid())
35+
{
36+
assert(_mh.native_handle()._init == native_handle()._init);
37+
}
38+
#endif
39+
OUTCOME_TRY(length, underlying_file_maximum_extent());
3440
if(length == 0)
3541
{
36-
// Not portable to map an empty file, so do nothing
42+
// Not portable to map an empty file, but retain reservation
3743
_reservation = reservation;
38-
return success();
3944
}
4045
if(reservation == 0)
4146
{
@@ -70,6 +75,7 @@ result<void> mapped_file_handle::close() noexcept
7075
LLFIO_LOG_FUNCTION_CALL(this);
7176
if(_mh.is_valid())
7277
{
78+
assert(_mh.native_handle()._init == native_handle()._init);
7379
OUTCOME_TRYV(_mh.close());
7480
}
7581
if(_sh.is_valid())
@@ -83,6 +89,7 @@ native_handle_type mapped_file_handle::release() noexcept
8389
LLFIO_LOG_FUNCTION_CALL(this);
8490
if(_mh.is_valid())
8591
{
92+
assert(_mh.native_handle()._init == native_handle()._init);
8693
(void) _mh.close();
8794
}
8895
if(_sh.is_valid())
@@ -95,6 +102,12 @@ native_handle_type mapped_file_handle::release() noexcept
95102
result<mapped_file_handle::extent_type> mapped_file_handle::truncate(extent_type newsize) noexcept
96103
{
97104
LLFIO_LOG_FUNCTION_CALL(this);
105+
#ifndef NDEBUG
106+
if(_mh.is_valid())
107+
{
108+
assert(_mh.native_handle()._init == native_handle()._init);
109+
}
110+
#endif
98111
// Release all maps and sections and truncate the backing file to zero
99112
if(newsize == 0)
100113
{
@@ -142,6 +155,12 @@ result<mapped_file_handle::extent_type> mapped_file_handle::truncate(extent_type
142155

143156
result<mapped_file_handle::extent_type> mapped_file_handle::update_map() noexcept
144157
{
158+
#ifndef NDEBUG
159+
if(_mh.is_valid())
160+
{
161+
assert(_mh.native_handle()._init == native_handle()._init);
162+
}
163+
#endif
145164
OUTCOME_TRY(auto &&length, underlying_file_maximum_extent());
146165
if(length > _reservation)
147166
{
@@ -156,12 +175,47 @@ result<mapped_file_handle::extent_type> mapped_file_handle::update_map() noexcep
156175
}
157176
if(!_sh.is_valid())
158177
{
159-
OUTCOME_TRYV(reserve(_reservation));
178+
(void) reserve(_reservation);
160179
return length;
161180
}
162181
// Adjust the map to reflect the new size of the section
163182
_mh._length = length;
164183
return length;
165184
}
166185

186+
result<void> mapped_file_handle::relink(const path_handle &base, path_view_type path, bool atomic_replace, deadline d) noexcept
187+
{
188+
#ifndef NDEBUG
189+
if(_mh.is_valid())
190+
{
191+
assert(_mh.native_handle()._init == native_handle()._init);
192+
}
193+
#endif
194+
// On POSIX relink() may change _v.fd sometimes.
195+
auto ret = file_handle::relink(base, path, atomic_replace, d);
196+
bool restore_maps = false;
197+
if(_sh.is_valid() && _v.fd != _sh.native_handle().fd)
198+
{
199+
OUTCOME_TRY(_sh.close());
200+
restore_maps = true;
201+
}
202+
if(_mh.is_valid() && _v.fd != _mh.native_handle().fd)
203+
{
204+
OUTCOME_TRY(_mh.close());
205+
restore_maps = true;
206+
}
207+
if(restore_maps)
208+
{
209+
OUTCOME_TRY(reserve(_reservation));
210+
}
211+
#ifndef NDEBUG
212+
if(_mh.is_valid())
213+
{
214+
assert(_mh.native_handle()._init == native_handle()._init);
215+
}
216+
#endif
217+
return ret;
218+
}
219+
220+
167221
LLFIO_V2_NAMESPACE_END

include/llfio/v2.0/detail/impl/windows/mapped_file_handle.ipp

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* An mapped handle to a file
2-
(C) 2017 Niall Douglas <http://www.nedproductions.biz/> (11 commits)
2+
(C) 2017-2021 Niall Douglas <http://www.nedproductions.biz/> (11 commits)
33
File Created: Sept 2017
44
55
@@ -27,15 +27,20 @@ Distributed under the Boost Software License, Version 1.0.
2727

2828
LLFIO_V2_NAMESPACE_BEGIN
2929

30-
result<mapped_file_handle::size_type> mapped_file_handle::reserve(size_type reservation) noexcept
30+
result<mapped_file_handle::size_type> mapped_file_handle::_reserve(extent_type &length, size_type reservation) noexcept
3131
{
3232
LLFIO_LOG_FUNCTION_CALL(this);
33-
OUTCOME_TRY(auto &&length, underlying_file_maximum_extent());
33+
#ifndef NDEBUG
34+
if(_mh.is_valid())
35+
{
36+
assert(_mh.native_handle()._init == native_handle()._init);
37+
}
38+
#endif
39+
OUTCOME_TRY(length, underlying_file_maximum_extent());
3440
if(length == 0)
3541
{
36-
// Cannot create a section nor a map for a zero lengthed file
42+
// Not portable to map an empty file, but retain reservation
3743
_reservation = reservation;
38-
return success();
3944
}
4045
if(reservation == 0)
4146
{
@@ -80,6 +85,7 @@ result<void> mapped_file_handle::close() noexcept
8085
LLFIO_LOG_FUNCTION_CALL(this);
8186
if(_mh.is_valid())
8287
{
88+
assert(_mh.native_handle()._init == native_handle()._init);
8389
OUTCOME_TRYV(_mh.close());
8490
}
8591
if(_sh.is_valid())
@@ -93,6 +99,7 @@ native_handle_type mapped_file_handle::release() noexcept
9399
LLFIO_LOG_FUNCTION_CALL(this);
94100
if(_mh.is_valid())
95101
{
102+
assert(_mh.native_handle()._init == native_handle()._init);
96103
(void) _mh.close();
97104
}
98105
if(_sh.is_valid())
@@ -105,6 +112,12 @@ native_handle_type mapped_file_handle::release() noexcept
105112
result<mapped_file_handle::extent_type> mapped_file_handle::truncate(extent_type newsize) noexcept
106113
{
107114
LLFIO_LOG_FUNCTION_CALL(this);
115+
#ifndef NDEBUG
116+
if(_mh.is_valid())
117+
{
118+
assert(_mh.native_handle()._init == native_handle()._init);
119+
}
120+
#endif
108121
// Release all maps and sections and truncate the backing file to zero
109122
if(newsize == 0)
110123
{
@@ -161,6 +174,12 @@ result<mapped_file_handle::extent_type> mapped_file_handle::truncate(extent_type
161174

162175
result<mapped_file_handle::extent_type> mapped_file_handle::update_map() noexcept
163176
{
177+
#ifndef NDEBUG
178+
if(_mh.is_valid())
179+
{
180+
assert(_mh.native_handle()._init == native_handle()._init);
181+
}
182+
#endif
164183
OUTCOME_TRY(auto &&length, underlying_file_maximum_extent());
165184
if(length > _reservation)
166185
{
@@ -175,7 +194,7 @@ result<mapped_file_handle::extent_type> mapped_file_handle::update_map() noexcep
175194
}
176195
if(!_sh.is_valid())
177196
{
178-
OUTCOME_TRYV(reserve(_reservation));
197+
(void) reserve(_reservation);
179198
return length;
180199
}
181200
OUTCOME_TRY(auto &&size, _sh.length());
@@ -193,4 +212,38 @@ result<mapped_file_handle::extent_type> mapped_file_handle::update_map() noexcep
193212
return length;
194213
}
195214

215+
result<void> mapped_file_handle::relink(const path_handle &base, path_view_type path, bool atomic_replace, deadline d) noexcept
216+
{
217+
#ifndef NDEBUG
218+
if(_mh.is_valid())
219+
{
220+
assert(_mh.native_handle()._init == native_handle()._init);
221+
}
222+
#endif
223+
// On Windows relink() won't succeed unless all maps are torn down
224+
bool restore_maps = false;
225+
if(_sh.is_valid())
226+
{
227+
OUTCOME_TRY(_sh.close());
228+
restore_maps = true;
229+
}
230+
if(_mh.is_valid())
231+
{
232+
OUTCOME_TRY(_mh.close());
233+
restore_maps = true;
234+
}
235+
auto ret = file_handle::relink(base, path, atomic_replace, d);
236+
if(restore_maps)
237+
{
238+
OUTCOME_TRY(reserve(_reservation));
239+
}
240+
#ifndef NDEBUG
241+
if(_mh.is_valid())
242+
{
243+
assert(_mh.native_handle()._init == native_handle()._init);
244+
}
245+
#endif
246+
return ret;
247+
}
248+
196249
LLFIO_V2_NAMESPACE_END

0 commit comments

Comments
 (0)