Skip to content

Commit 17214b7

Browse files
committed
Merge tag 'fsverity-for-linus' of git://git.kernel.org/pub/scm/fs/fsverity/linux
Pull fsverity fixes from Eric Biggers: "Fix two significant performance issues with fsverity" * tag 'fsverity-for-linus' of git://git.kernel.org/pub/scm/fs/fsverity/linux: fsverity: don't drop pagecache at end of FS_IOC_ENABLE_VERITY fsverity: Remove WQ_UNBOUND from fsverity read workqueue
2 parents 4f1e308 + a075bac commit 17214b7

File tree

2 files changed

+19
-18
lines changed

2 files changed

+19
-18
lines changed

fs/verity/enable.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "fsverity_private.h"
99

1010
#include <linux/mount.h>
11-
#include <linux/pagemap.h>
1211
#include <linux/sched/signal.h>
1312
#include <linux/uaccess.h>
1413

@@ -367,25 +366,27 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
367366
goto out_drop_write;
368367

369368
err = enable_verity(filp, &arg);
370-
if (err)
371-
goto out_allow_write_access;
372369

373370
/*
374-
* Some pages of the file may have been evicted from pagecache after
375-
* being used in the Merkle tree construction, then read into pagecache
376-
* again by another process reading from the file concurrently. Since
377-
* these pages didn't undergo verification against the file digest which
378-
* fs-verity now claims to be enforcing, we have to wipe the pagecache
379-
* to ensure that all future reads are verified.
371+
* We no longer drop the inode's pagecache after enabling verity. This
372+
* used to be done to try to avoid a race condition where pages could be
373+
* evicted after being used in the Merkle tree construction, then
374+
* re-instantiated by a concurrent read. Such pages are unverified, and
375+
* the backing storage could have filled them with different content, so
376+
* they shouldn't be used to fulfill reads once verity is enabled.
377+
*
378+
* But, dropping the pagecache has a big performance impact, and it
379+
* doesn't fully solve the race condition anyway. So for those reasons,
380+
* and also because this race condition isn't very important relatively
381+
* speaking (especially for small-ish files, where the chance of a page
382+
* being used, evicted, *and* re-instantiated all while enabling verity
383+
* is quite small), we no longer drop the inode's pagecache.
380384
*/
381-
filemap_write_and_wait(inode->i_mapping);
382-
invalidate_inode_pages2(inode->i_mapping);
383385

384386
/*
385387
* allow_write_access() is needed to pair with deny_write_access().
386388
* Regardless, the filesystem won't allow writing to verity files.
387389
*/
388-
out_allow_write_access:
389390
allow_write_access(filp);
390391
out_drop_write:
391392
mnt_drop_write_file(filp);

fs/verity/verify.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -387,15 +387,15 @@ EXPORT_SYMBOL_GPL(fsverity_enqueue_verify_work);
387387
int __init fsverity_init_workqueue(void)
388388
{
389389
/*
390-
* Use an unbound workqueue to allow bios to be verified in parallel
391-
* even when they happen to complete on the same CPU. This sacrifices
392-
* locality, but it's worthwhile since hashing is CPU-intensive.
390+
* Use a high-priority workqueue to prioritize verification work, which
391+
* blocks reads from completing, over regular application tasks.
393392
*
394-
* Also use a high-priority workqueue to prioritize verification work,
395-
* which blocks reads from completing, over regular application tasks.
393+
* For performance reasons, don't use an unbound workqueue. Using an
394+
* unbound workqueue for crypto operations causes excessive scheduler
395+
* latency on ARM64.
396396
*/
397397
fsverity_read_workqueue = alloc_workqueue("fsverity_read_queue",
398-
WQ_UNBOUND | WQ_HIGHPRI,
398+
WQ_HIGHPRI,
399399
num_online_cpus());
400400
if (!fsverity_read_workqueue)
401401
return -ENOMEM;

0 commit comments

Comments
 (0)