| Message ID | 927dbeab576537ac0e4c954a8c3d13b148cd6a26.1662420176.git.sweettea-kernel@dorminy.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add fscrypt integration | expand |
Hi Sweet,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on kdave/for-next]
[also build test ERROR on next-20220901]
[cannot apply to linus/master v6.0-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sweet-Tea-Dorminy/btrfs-add-fscrypt-integration/20220906-101551
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: riscv-randconfig-r042-20220906
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/69ed072dd1465ef791ed9ca45b84c27edfd45492
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sweet-Tea-Dorminy/btrfs-add-fscrypt-integration/20220906-101551
git checkout 69ed072dd1465ef791ed9ca45b84c27edfd45492
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
fs/btrfs/reflink.c: In function 'btrfs_remap_file_range_prep':
>> fs/btrfs/reflink.c:811:14: error: implicit declaration of function 'fscrypt_have_same_policy'; did you mean 'fscrypt_free_dummy_policy'? [-Werror=implicit-function-declaration]
811 | if (!fscrypt_have_same_policy(inode_in, inode_out)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~
| fscrypt_free_dummy_policy
cc1: some warnings being treated as errors
vim +811 fs/btrfs/reflink.c
788
789 static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
790 struct file *file_out, loff_t pos_out,
791 loff_t *len, unsigned int remap_flags)
792 {
793 struct inode *inode_in = file_inode(file_in);
794 struct inode *inode_out = file_inode(file_out);
795 u64 bs = BTRFS_I(inode_out)->root->fs_info->sb->s_blocksize;
796 u64 wb_len;
797 int ret;
798
799 if (!(remap_flags & REMAP_FILE_DEDUP)) {
800 struct btrfs_root *root_out = BTRFS_I(inode_out)->root;
801
802 if (btrfs_root_readonly(root_out))
803 return -EROFS;
804
805 ASSERT(inode_in->i_sb == inode_out->i_sb);
806 }
807
808 /*
809 * Can only reflink encrypted files if both files are encrypted.
810 */
> 811 if (!fscrypt_have_same_policy(inode_in, inode_out)) {
812 return -EINVAL;
813 }
814
815 /* Don't make the dst file partly checksummed */
816 if ((BTRFS_I(inode_in)->flags & BTRFS_INODE_NODATASUM) !=
817 (BTRFS_I(inode_out)->flags & BTRFS_INODE_NODATASUM)) {
818 return -EINVAL;
819 }
820
821 /*
822 * Now that the inodes are locked, we need to start writeback ourselves
823 * and can not rely on the writeback from the VFS's generic helper
824 * generic_remap_file_range_prep() because:
825 *
826 * 1) For compression we must call filemap_fdatawrite_range() range
827 * twice (btrfs_fdatawrite_range() does it for us), and the generic
828 * helper only calls it once;
829 *
830 * 2) filemap_fdatawrite_range(), called by the generic helper only
831 * waits for the writeback to complete, i.e. for IO to be done, and
832 * not for the ordered extents to complete. We need to wait for them
833 * to complete so that new file extent items are in the fs tree.
834 */
835 if (*len == 0 && !(remap_flags & REMAP_FILE_DEDUP))
836 wb_len = ALIGN(inode_in->i_size, bs) - ALIGN_DOWN(pos_in, bs);
837 else
838 wb_len = ALIGN(*len, bs);
839
840 /*
841 * Workaround to make sure NOCOW buffered write reach disk as NOCOW.
842 *
843 * Btrfs' back references do not have a block level granularity, they
844 * work at the whole extent level.
845 * NOCOW buffered write without data space reserved may not be able
846 * to fall back to CoW due to lack of data space, thus could cause
847 * data loss.
848 *
849 * Here we take a shortcut by flushing the whole inode, so that all
850 * nocow write should reach disk as nocow before we increase the
851 * reference of the extent. We could do better by only flushing NOCOW
852 * data, but that needs extra accounting.
853 *
854 * Also we don't need to check ASYNC_EXTENT, as async extent will be
855 * CoWed anyway, not affecting nocow part.
856 */
857 ret = filemap_flush(inode_in->i_mapping);
858 if (ret < 0)
859 return ret;
860
861 ret = btrfs_wait_ordered_range(inode_in, ALIGN_DOWN(pos_in, bs),
862 wb_len);
863 if (ret < 0)
864 return ret;
865 ret = btrfs_wait_ordered_range(inode_out, ALIGN_DOWN(pos_out, bs),
866 wb_len);
867 if (ret < 0)
868 return ret;
869
870 return generic_remap_file_range_prep(file_in, pos_in, file_out, pos_out,
871 len, remap_flags);
872 }
873
On Mon, Sep 05, 2022 at 08:35:26PM -0400, Sweet Tea Dorminy wrote: > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -1895,7 +1895,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) > goto relock; > } > > - if (check_direct_IO(fs_info, from, pos)) { > + if (IS_ENCRYPTED(inode) || check_direct_IO(fs_info, from, pos)) { I'm not sure if we want the IS_ENCRYPTED as an explicit check or put it to check_direct_IO, but probably to make it obvious that direct io does not work with it a separate check is ok. > btrfs_inode_unlock(inode, ilock_flags); > goto buffered; > } > @@ -3729,7 +3729,7 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) > ssize_t read = 0; > ssize_t ret; > > - if (fsverity_active(inode)) > + if (IS_ENCRYPTED(inode) || fsverity_active(inode)) Yeah as we have something similar for verity. > return 0; > > if (check_direct_read(btrfs_sb(inode->i_sb), to, iocb->ki_pos)) > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 482c5b3d9e70..fea48c12a33a 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -409,7 +409,8 @@ static noinline int cow_file_range_inline(struct btrfs_inode *inode, u64 size, > * compressed) data fits in a leaf and the configured maximum inline > * size. > */ > - if (size < i_size_read(&inode->vfs_inode) || > + if (IS_ENCRYPTED(&inode->vfs_inode) || > + size < i_size_read(&inode->vfs_inode) || > size > fs_info->sectorsize || > data_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info) || > data_len > fs_info->max_inline) > diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c > index 9acf47b11fe6..d22086e1cbc8 100644 > --- a/fs/btrfs/reflink.c > +++ b/fs/btrfs/reflink.c > @@ -805,6 +805,13 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, > ASSERT(inode_in->i_sb == inode_out->i_sb); > } > > + /* > + * Can only reflink encrypted files if both files are encrypted. > + */ The comment fits on one line (and slight 80 chars overflow is ok). > + if (!fscrypt_have_same_policy(inode_in, inode_out)) { > + return -EINVAL; Single statements don't need the { } > + } > +
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h index b160b8e124e0..ff668686717b 100644 --- a/fs/btrfs/btrfs_inode.h +++ b/fs/btrfs/btrfs_inode.h @@ -400,6 +400,9 @@ static inline bool btrfs_inode_in_log(struct btrfs_inode *inode, u64 generation) */ static inline bool btrfs_inode_can_compress(const struct btrfs_inode *inode) { + if (IS_ENCRYPTED(&inode->vfs_inode)) + return false; + if (inode->flags & BTRFS_INODE_NODATACOW || inode->flags & BTRFS_INODE_NODATASUM) return false; diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 0a76ae8b8e96..7216ac1f860c 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1895,7 +1895,7 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from) goto relock; } - if (check_direct_IO(fs_info, from, pos)) { + if (IS_ENCRYPTED(inode) || check_direct_IO(fs_info, from, pos)) { btrfs_inode_unlock(inode, ilock_flags); goto buffered; } @@ -3729,7 +3729,7 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) ssize_t read = 0; ssize_t ret; - if (fsverity_active(inode)) + if (IS_ENCRYPTED(inode) || fsverity_active(inode)) return 0; if (check_direct_read(btrfs_sb(inode->i_sb), to, iocb->ki_pos)) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 482c5b3d9e70..fea48c12a33a 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -409,7 +409,8 @@ static noinline int cow_file_range_inline(struct btrfs_inode *inode, u64 size, * compressed) data fits in a leaf and the configured maximum inline * size. */ - if (size < i_size_read(&inode->vfs_inode) || + if (IS_ENCRYPTED(&inode->vfs_inode) || + size < i_size_read(&inode->vfs_inode) || size > fs_info->sectorsize || data_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info) || data_len > fs_info->max_inline) diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c index 9acf47b11fe6..d22086e1cbc8 100644 --- a/fs/btrfs/reflink.c +++ b/fs/btrfs/reflink.c @@ -805,6 +805,13 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, ASSERT(inode_in->i_sb == inode_out->i_sb); } + /* + * Can only reflink encrypted files if both files are encrypted. + */ + if (!fscrypt_have_same_policy(inode_in, inode_out)) { + return -EINVAL; + } + /* Don't make the dst file partly checksummed */ if ((BTRFS_I(inode_in)->flags & BTRFS_INODE_NODATASUM) != (BTRFS_I(inode_out)->flags & BTRFS_INODE_NODATASUM)) {