Skip to content

Commit 8ec7a51

Browse files
dberlindberlin
committed
Update for proper locking
1 parent f2835e7 commit 8ec7a51

File tree

3 files changed

+60
-65
lines changed

3 files changed

+60
-65
lines changed

‎include/os/linux/zfs/sys/zpl.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,24 +190,22 @@ typedef struct zfs_locked_range zfs_locked_range_t;
190190
/* handlers for file_operations of the same name */
191191
extern ssize_t
192192
zpl_copy_file_range(struct file *src_file, loff_t src_off,
193-
struct file *dst_file, loff_t dst_off, size_t len,
193+
struct file *dst_file, loff_t dst_off, size_t len,
194194
unsigned int flags);
195195
extern loff_t
196196
zpl_remap_file_range(struct file *src_file, loff_t src_off,
197-
struct file *dst_file, loff_t dst_off, loff_t len,
198-
unsigned int flags);
197+
struct file *dst_file, loff_t dst_off, loff_t len,
198+
unsigned int flags);
199199
extern int
200200
zpl_clone_file_range(struct file *src_file, loff_t src_off,
201-
struct file *dst_file, loff_t dst_off, uint64_t len);
201+
struct file *dst_file, loff_t dst_off, uint64_t len);
202202
extern int
203203
zpl_dedupe_file_range(struct file *src_file, loff_t src_off,
204-
struct file *dst_file, loff_t dst_off, uint64_t len);
204+
struct file *dst_file, loff_t dst_off, uint64_t len);
205205
extern int
206206
zpl_dedupe_file_compare_locked(struct file *src_file, loff_t src_off,
207-
struct file *dst_file, loff_t dst_off,
208-
uint64_t len, bool *is_same,
209-
zfs_locked_range_t *src_zlr,
210-
zfs_locked_range_t *dst_zlr);
207+
struct file *dst_file, loff_t dst_off, uint64_t len, bool *is_same,
208+
zfs_locked_range_t *src_zlr, zfs_locked_range_t *dst_zlr);
211209

212210
/* compat for FICLONE/FICLONERANGE/FIDEDUPERANGE ioctls */
213211
typedef struct {

‎module/os/linux/zfs/zpl_file_range.c

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,9 @@ __zpl_clone_file_range(struct file *src_file, loff_t src_off,
8787
*/
8888
static ssize_t
8989
__zpl_clone_file_range_locked(struct file *src_file, loff_t src_off,
90-
struct file *dst_file, loff_t dst_off, size_t len,
91-
zfs_locked_range_t *src_zlr, zfs_locked_range_t *dst_zlr) {
90+
struct file *dst_file, loff_t dst_off, size_t len,
91+
zfs_locked_range_t *src_zlr, zfs_locked_range_t *dst_zlr)
92+
{
9293
struct inode *src_i = file_inode(src_file);
9394
struct inode *dst_i = file_inode(dst_file);
9495
uint64_t src_off_o = (uint64_t)src_off;
@@ -98,8 +99,8 @@ __zpl_clone_file_range_locked(struct file *src_file, loff_t src_off,
9899
fstrans_cookie_t cookie;
99100
int err;
100101

101-
if (!spa_feature_is_enabled(dmu_objset_spa(ITOZSB(dst_i)->z_os),
102-
SPA_FEATURE_BLOCK_CLONING))
102+
if (!spa_feature_is_enabled(
103+
dmu_objset_spa(ITOZSB(dst_i)->z_os), SPA_FEATURE_BLOCK_CLONING))
103104
return (-EOPNOTSUPP);
104105

105106
if (src_i != dst_i)
@@ -108,9 +109,8 @@ __zpl_clone_file_range_locked(struct file *src_file, loff_t src_off,
108109

109110
crhold(cr);
110111
cookie = spl_fstrans_mark();
111-
112-
err = -zfs_clone_range_locked(ITOZ(src_i), &src_off_o, ITOZ(dst_i), &dst_off_o,
113-
&len_o, cr, src_zlr, dst_zlr);
112+
err = -zfs_clone_range_locked(ITOZ(src_i), &src_off_o, ITOZ(dst_i),
113+
&dst_off_o, &len_o, cr, src_zlr, dst_zlr);
114114

115115
spl_fstrans_unmark(cookie);
116116
crfree(cr);
@@ -125,7 +125,6 @@ __zpl_clone_file_range_locked(struct file *src_file, loff_t src_off,
125125
return ((ssize_t)len_o);
126126
}
127127

128-
129128
#if defined(HAVE_VFS_COPY_FILE_RANGE) || \
130129
defined(HAVE_VFS_FILE_OPERATIONS_EXTEND)
131130
/*
@@ -196,11 +195,6 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off,
196195
* doesn't do that, so we just turn the flag off.
197196
*/
198197
flags &= ~REMAP_FILE_CAN_SHORTEN;
199-
/* Zero length means to clone everything to the end of the file
200-
*/
201-
if (len == 0)
202-
len = i_size_read(file_inode(src_file)) - src_off;
203-
204198
if (flags & REMAP_FILE_DEDUP) {
205199
/* Both nodes must be range locked */
206200
zfs_locked_range_t *src_zlr;
@@ -229,9 +223,8 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off,
229223
}
230224

231225
bool same = false;
232-
int ret = zpl_dedupe_file_compare_locked(
233-
src_file, src_off, dst_file, dst_off, len, &same, src_zlr,
234-
dst_zlr);
226+
int ret = zpl_dedupe_file_compare_locked(src_file, src_off,
227+
dst_file, dst_off, len, &same, src_zlr, dst_zlr);
235228
if (ret) {
236229
goto cleanup;
237230
}
@@ -240,11 +233,11 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off,
240233
goto cleanup;
241234
}
242235
ret = __zpl_clone_file_range_locked(src_file, src_off, dst_file,
243-
dst_off, len, src_zlr, dst_zlr);
236+
dst_off, len, src_zlr, dst_zlr);
244237
cleanup:
245238
zfs_rangelock_exit(src_zlr);
246239
zfs_rangelock_exit(dst_zlr);
247-
return ret;
240+
return (ret);
248241
}
249242
if (len == 0)
250243
len = i_size_read(file_inode(src_file)) - src_off;
@@ -261,7 +254,8 @@ zpl_remap_file_range(struct file *src_file, loff_t src_off,
261254
*/
262255
int
263256
zpl_clone_file_range(struct file *src_file, loff_t src_off,
264-
struct file *dst_file, loff_t dst_off, uint64_t len) {
257+
struct file *dst_file, loff_t dst_off, uint64_t len)
258+
{
265259
/* Zero length means to clone everything to the end of the file */
266260
if (len == 0)
267261
len = i_size_read(file_inode(src_file)) - src_off;
@@ -358,36 +352,36 @@ zpl_ioctl_ficlonerange(struct file *dst_file, void __user *arg)
358352

359353
/* Entry point for FIDEDUPERANGE, before Linux 4.5. */
360354
long
361-
zpl_ioctl_fideduperange(struct file *src_file, void *arg) {
355+
zpl_ioctl_fideduperange(struct file *src_file, void *arg)
356+
{
362357
zfs_ioc_compat_dedupe_range_t dup;
363358
int i;
364-
365-
if (copy_from_user(&dup, arg, sizeof(dup)))
359+
if (copy_from_user(&dup, arg, sizeof (dup)))
366360
return (-EFAULT);
367361

368362
u16 count = dup.fdr_dest_count;
369363
struct inode *src_inode = file_inode(src_file);
370364

371365
/* Nothing to duplicate to */
372366
if (count == 0)
373-
return -EINVAL;
367+
return (-EINVAL);
374368

375369
/* Check the src file */
376370
if (!(src_file->f_mode & FMODE_READ))
377-
return -EINVAL;
371+
return (-EINVAL);
378372

379373
if (S_ISDIR(src_inode->i_mode))
380-
return -EISDIR;
374+
return (-EISDIR);
381375

382376
if (!S_ISREG(src_inode->i_mode))
383-
return -EINVAL;
377+
return (-EINVAL);
384378

385379
if (dup.fdr_src_offset + dup.fdr_src_length > i_size_read(src_inode))
386-
return -EINVAL;
380+
return (-EINVAL);
387381

388382
/* Check the dup structure */
389383
if (dup.fdr_reserved1 || dup.fdr_reserved2)
390-
return -EINVAL;
384+
return (-EINVAL);
391385

392386
/* Set output values to safe results */
393387
for (i = 0; i < count; i++) {
@@ -409,8 +403,8 @@ zpl_ioctl_fideduperange(struct file *src_file, void *arg) {
409403
}
410404
loff_t deduped =
411405
zpl_remap_file_range(src_file, dup.fdr_src_offset, dst_file,
412-
dup.fdr_info[i].fdri_dest_offset,
413-
dup.fdr_src_length, REMAP_FILE_DEDUP);
406+
dup.fdr_info[i].fdri_dest_offset, dup.fdr_src_length,
407+
REMAP_FILE_DEDUP);
414408
if (deduped == -EBADE) {
415409
dup.fdr_info[i].fdri_status = FILE_DEDUPE_RANGE_DIFFERS;
416410
} else if (deduped < 0) {
@@ -421,15 +415,14 @@ zpl_ioctl_fideduperange(struct file *src_file, void *arg) {
421415
do_fdput:
422416
fdput(dst_fd);
423417
}
424-
return 0;
418+
return (0);
425419
}
426420

427421
int
428422
zpl_dedupe_file_compare_locked(struct file *src_file, loff_t src_off,
429-
struct file *dst_file, loff_t dst_off,
430-
uint64_t len, bool *is_same,
431-
zfs_locked_range_t *src_zlr,
432-
zfs_locked_range_t *dst_zlr) {
423+
struct file *dst_file, loff_t dst_off, uint64_t len, bool *is_same,
424+
zfs_locked_range_t *src_zlr, zfs_locked_range_t *dst_zlr)
425+
{
433426
bool same = true;
434427
int err = 0;
435428
znode_t *src_znode = ITOZ(file_inode(src_file));
@@ -443,7 +436,7 @@ zpl_dedupe_file_compare_locked(struct file *src_file, loff_t src_off,
443436
zfs_uio_t uio;
444437

445438
uint64_t cmp_len = min(PAGE_SIZE - offset_in_page(src_off),
446-
PAGE_SIZE - offset_in_page(dst_off));
439+
PAGE_SIZE - offset_in_page(dst_off));
447440

448441
cmp_len = min(cmp_len, len);
449442

@@ -456,25 +449,29 @@ zpl_dedupe_file_compare_locked(struct file *src_file, loff_t src_off,
456449
zfs_uio_iovec_init(&uio, &iov, 1, 0, UIO_SYSSPACE, cmp_len, 0);
457450
crhold(cr);
458451
cookie = spl_fstrans_mark();
459-
err = -zfs_read_locked(src_znode, &uio, src_file->f_flags, cr,
460-
src_zlr);
452+
err = -zfs_read_locked(
453+
src_znode, &uio, src_file->f_flags, cr, src_zlr);
461454
spl_fstrans_unmark(cookie);
462455
crfree(cr);
463-
456+
if (zfs_uio_resid(&uio) != 0)
457+
err = -EIO;
464458
if (err)
465459
goto done;
466460
iov.iov_base = dst_buf;
467461
iov.iov_len = cmp_len;
462+
463+
zfs_uio_iovec_init(&uio, &iov, 1, 0, UIO_SYSSPACE, cmp_len, 0);
468464
crhold(cr);
469465
cookie = spl_fstrans_mark();
470-
err = -zfs_read_locked(dst_znode, &uio, dst_file->f_flags, cr,
471-
dst_zlr);
466+
err = -zfs_read_locked(
467+
dst_znode, &uio, dst_file->f_flags, cr, dst_zlr);
472468
spl_fstrans_unmark(cookie);
473469
crfree(cr);
470+
if (zfs_uio_resid(&uio) != 0)
471+
err = -EIO;
474472

475473
if (err)
476474
goto done;
477-
478475
if (memcmp(src_buf, dst_buf, cmp_len))
479476
same = false;
480477

@@ -487,9 +484,8 @@ zpl_dedupe_file_compare_locked(struct file *src_file, loff_t src_off,
487484
}
488485

489486
*is_same = same;
490-
491487
done:
492488
kmem_free(src_buf, PAGE_SIZE);
493489
kmem_free(dst_buf, PAGE_SIZE);
494-
return err;
490+
return (err);
495491
}

‎module/zfs/zfs_vnops.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -354,15 +354,15 @@ zfs_read(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
354354
*/
355355
int
356356
zfs_read_locked(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr,
357-
zfs_locked_range_t *zrl)
357+
zfs_locked_range_t *zrl)
358358
{
359359
(void) cr;
360360
int error = 0;
361361
boolean_t frsync = B_FALSE;
362362

363363
/* The range is verified below */
364364
if (!zrl) {
365-
return -EINVAL;
365+
return (-EINVAL);
366366
}
367367

368368
zfsvfs_t *zfsvfs = ZTOZSB(zp);
@@ -412,11 +412,12 @@ zfs_read_locked(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr,
412412
zil_commit(zfsvfs->z_log, zp->z_id);
413413

414414
/*
415-
* Ensure the lock covers the range.
415+
* Ensure the lock covers the range.
416416
*/
417417
if (zrl->lr_offset > zfs_uio_offset(uio) ||
418-
(zfs_uio_offset(uio) + zfs_uio_resid(uio)) > (zrl->lr_offset + zrl->lr_length)) {
419-
return -EINVAL;
418+
(zfs_uio_offset(uio) + zfs_uio_resid(uio)) >
419+
(zrl->lr_offset + zrl->lr_length)) {
420+
return (-EINVAL);
420421
}
421422
/*
422423
* If we are reading past end-of-file we can skip
@@ -1550,10 +1551,10 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
15501551
/*
15511552
* This function is exactly the same as zfs_clone_range, except
15521553
* it is called with the range locks already held.
1553-
*
1554+
*
15541555
* It verifies that we hold locks for the src/dest clone range.
15551556
* It performs no unlocking of the locks.
1556-
*
1557+
*
15571558
* On success, the function return the number of bytes copied in *lenp.
15581559
* Note, it doesn't return how much bytes are left to be copied.
15591560
* On errors which are caused by any file system limitations or
@@ -1563,8 +1564,8 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
15631564
*/
15641565
int
15651566
zfs_clone_range_locked(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
1566-
uint64_t *outoffp, uint64_t *lenp, cred_t *cr,
1567-
zfs_locked_range_t *src_zlr, zfs_locked_range_t *dst_zlr)
1567+
uint64_t *outoffp, uint64_t *lenp, cred_t *cr, zfs_locked_range_t *src_zlr,
1568+
zfs_locked_range_t *dst_zlr)
15681569
{
15691570
zfsvfs_t *inzfsvfs, *outzfsvfs;
15701571
objset_t *inos, *outos;
@@ -1686,15 +1687,15 @@ zfs_clone_range_locked(znode_t *inzp, uint64_t *inoffp, znode_t *outzp,
16861687
* Verify locks
16871688
*/
16881689
if (!src_zlr || !dst_zlr || dst_zlr->lr_type != RL_WRITER) {
1689-
return -EINVAL;
1690+
return (-EINVAL);
16901691
}
16911692
if (src_zlr->lr_offset > inoff ||
16921693
(inoff + len) > (src_zlr->lr_offset + src_zlr->lr_length)) {
1693-
return -EINVAL;
1694+
return (-EINVAL);
16941695
}
16951696
if (dst_zlr->lr_offset > outoff ||
16961697
(outoff + len) > (dst_zlr->lr_offset + dst_zlr->lr_length)) {
1697-
return -EINVAL;
1698+
return (-EINVAL);
16981699
}
16991700

17001701
inblksz = inzp->z_blksz;

0 commit comments

Comments
 (0)