From: Eric Biggers <ebiggers@kernel.org> To: Satya Tangirala <satyat@google.com> Cc: linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-ext4@vger.kernel.org, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com> Subject: Re: [PATCH v7 7/9] fscrypt: add inline encryption support Date: Fri, 21 Feb 2020 10:40:09 -0800 [thread overview] Message-ID: <20200221184009.GD925@sol.localdomain> (raw) In-Reply-To: <20200221115050.238976-8-satyat@google.com> On Fri, Feb 21, 2020 at 03:50:48AM -0800, Satya Tangirala wrote: > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c > index 4fa18fff9c4e..82d06cf4b94a 100644 > --- a/fs/crypto/bio.c > +++ b/fs/crypto/bio.c > @@ -24,6 +24,8 @@ > #include <linux/module.h> > #include <linux/bio.h> > #include <linux/namei.h> > +#include <linux/fscrypt.h> No need to include <linux/fscrypt.h> explicitly here, since everything in fs/crypto/ already gets it via "fscrypt_private.h". > +static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, > + pgoff_t lblk, sector_t pblk, > + unsigned int len) > +{ > + const unsigned int blockbits = inode->i_blkbits; > + const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits); > + struct bio *bio; > + int ret, err = 0; > + int num_pages = 0; > + > + /* This always succeeds since __GFP_DIRECT_RECLAIM is set. */ > + bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES); > + > + while (len) { > + unsigned int blocks_this_page = min(len, blocks_per_page); > + unsigned int bytes_this_page = blocks_this_page << blockbits; > + > + if (num_pages == 0) { > + fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOIO); This should use GFP_NOFS rather than the stricter GFP_NOIO. > + bio_set_dev(bio, inode->i_sb->s_bdev); > + bio->bi_iter.bi_sector = > + pblk << (blockbits - SECTOR_SHIFT); > + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > + } > + ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0); > + if (WARN_ON(ret != bytes_this_page)) { > + err = -EIO; > + goto out; > + } > + num_pages++; > + len -= blocks_this_page; > + lblk += blocks_this_page; > + pblk += blocks_this_page; > + if (num_pages == BIO_MAX_PAGES || !len) { > + err = submit_bio_wait(bio); > + if (!err && bio->bi_status) > + err = -EIO; submit_bio_wait() already checks bi_status and reflects it in the returned error, so checking it again here is redundant. > @@ -69,12 +119,17 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk, > unsigned int nr_pages; > unsigned int i; > unsigned int offset; > + const bool inlinecrypt = fscrypt_inode_uses_inline_crypto(inode); > struct bio *bio; > int ret, err; > > if (len == 0) > return 0; > > + if (inlinecrypt) > + return fscrypt_zeroout_range_inline_crypt(inode, lblk, pblk, > + len); > + No need for the 'inlinecrypt' bool variable. Just do: if (fscrypt_inode_uses_inline_crypto(inode)) FYI, I had suggested a merge resolution to use here which didn't have the above problems. Looks like you missed it? https://lkml.kernel.org/linux-block/20200114211243.GC41220@gmail.com/ - Eric
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org> To: Satya Tangirala <satyat@google.com> Cc: linux-scsi@vger.kernel.org, Kim Boojin <boojin.kim@samsung.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, linux-f2fs-devel@lists.sourceforge.net, linux-block@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org Subject: Re: [f2fs-dev] [PATCH v7 7/9] fscrypt: add inline encryption support Date: Fri, 21 Feb 2020 10:40:09 -0800 [thread overview] Message-ID: <20200221184009.GD925@sol.localdomain> (raw) In-Reply-To: <20200221115050.238976-8-satyat@google.com> On Fri, Feb 21, 2020 at 03:50:48AM -0800, Satya Tangirala wrote: > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c > index 4fa18fff9c4e..82d06cf4b94a 100644 > --- a/fs/crypto/bio.c > +++ b/fs/crypto/bio.c > @@ -24,6 +24,8 @@ > #include <linux/module.h> > #include <linux/bio.h> > #include <linux/namei.h> > +#include <linux/fscrypt.h> No need to include <linux/fscrypt.h> explicitly here, since everything in fs/crypto/ already gets it via "fscrypt_private.h". > +static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode, > + pgoff_t lblk, sector_t pblk, > + unsigned int len) > +{ > + const unsigned int blockbits = inode->i_blkbits; > + const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits); > + struct bio *bio; > + int ret, err = 0; > + int num_pages = 0; > + > + /* This always succeeds since __GFP_DIRECT_RECLAIM is set. */ > + bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES); > + > + while (len) { > + unsigned int blocks_this_page = min(len, blocks_per_page); > + unsigned int bytes_this_page = blocks_this_page << blockbits; > + > + if (num_pages == 0) { > + fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOIO); This should use GFP_NOFS rather than the stricter GFP_NOIO. > + bio_set_dev(bio, inode->i_sb->s_bdev); > + bio->bi_iter.bi_sector = > + pblk << (blockbits - SECTOR_SHIFT); > + bio_set_op_attrs(bio, REQ_OP_WRITE, 0); > + } > + ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0); > + if (WARN_ON(ret != bytes_this_page)) { > + err = -EIO; > + goto out; > + } > + num_pages++; > + len -= blocks_this_page; > + lblk += blocks_this_page; > + pblk += blocks_this_page; > + if (num_pages == BIO_MAX_PAGES || !len) { > + err = submit_bio_wait(bio); > + if (!err && bio->bi_status) > + err = -EIO; submit_bio_wait() already checks bi_status and reflects it in the returned error, so checking it again here is redundant. > @@ -69,12 +119,17 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk, > unsigned int nr_pages; > unsigned int i; > unsigned int offset; > + const bool inlinecrypt = fscrypt_inode_uses_inline_crypto(inode); > struct bio *bio; > int ret, err; > > if (len == 0) > return 0; > > + if (inlinecrypt) > + return fscrypt_zeroout_range_inline_crypt(inode, lblk, pblk, > + len); > + No need for the 'inlinecrypt' bool variable. Just do: if (fscrypt_inode_uses_inline_crypto(inode)) FYI, I had suggested a merge resolution to use here which didn't have the above problems. Looks like you missed it? https://lkml.kernel.org/linux-block/20200114211243.GC41220@gmail.com/ - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2020-02-21 18:40 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-21 11:50 [PATCH v7 0/9] Inline Encryption Support Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 11:50 ` [PATCH v7 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 17:04 ` Christoph Hellwig 2020-02-21 17:04 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 17:31 ` Christoph Hellwig 2020-02-21 17:31 ` [f2fs-dev] " Christoph Hellwig 2020-02-27 18:14 ` Eric Biggers 2020-02-27 18:14 ` [f2fs-dev] " Eric Biggers 2020-02-27 21:25 ` Satya Tangirala 2020-02-27 21:25 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-03-05 16:11 ` Christoph Hellwig 2020-03-05 16:11 ` [f2fs-dev] " Christoph Hellwig 2020-02-27 18:48 ` Eric Biggers 2020-02-27 18:48 ` [f2fs-dev] " Eric Biggers 2020-02-21 11:50 ` [PATCH v7 2/9] block: Inline encryption support for blk-mq Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 17:22 ` Christoph Hellwig 2020-02-21 17:22 ` [f2fs-dev] " Christoph Hellwig 2020-02-22 0:52 ` Satya Tangirala 2020-02-22 0:52 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-24 23:34 ` Christoph Hellwig 2020-02-24 23:34 ` [f2fs-dev] " Christoph Hellwig 2020-02-27 18:25 ` Eric Biggers 2020-02-27 18:25 ` [f2fs-dev] " Eric Biggers 2020-02-21 11:50 ` [PATCH v7 3/9] block: blk-crypto-fallback for Inline Encryption Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 16:51 ` Randy Dunlap 2020-02-21 16:51 ` [f2fs-dev] " Randy Dunlap 2020-02-21 17:25 ` Christoph Hellwig 2020-02-21 17:25 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 17:35 ` Christoph Hellwig 2020-02-21 17:35 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 18:34 ` Eric Biggers 2020-02-21 18:34 ` [f2fs-dev] " Eric Biggers 2020-02-24 23:36 ` Christoph Hellwig 2020-02-24 23:36 ` [f2fs-dev] " Christoph Hellwig 2020-02-27 19:25 ` Eric Biggers 2020-02-27 19:25 ` [f2fs-dev] " Eric Biggers 2020-02-21 11:50 ` [PATCH v7 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 11:50 ` [PATCH v7 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-22 4:59 ` Eric Biggers 2020-02-22 4:59 ` [f2fs-dev] " Eric Biggers 2020-02-21 11:50 ` [PATCH v7 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 17:22 ` Christoph Hellwig 2020-02-21 17:22 ` [f2fs-dev] " Christoph Hellwig 2020-02-21 18:11 ` Eric Biggers 2020-02-21 18:11 ` [f2fs-dev] " Eric Biggers 2020-02-23 13:47 ` Stanley Chu 2020-02-23 13:47 ` [f2fs-dev] " Stanley Chu 2020-02-24 23:37 ` Christoph Hellwig 2020-02-24 23:37 ` [f2fs-dev] " Christoph Hellwig 2020-02-25 7:21 ` Stanley Chu 2020-02-25 7:21 ` [f2fs-dev] " Stanley Chu 2020-02-26 1:12 ` Eric Biggers 2020-02-26 1:12 ` [f2fs-dev] " Eric Biggers 2020-02-26 6:43 ` Stanley Chu 2020-02-26 6:43 ` [f2fs-dev] " Stanley Chu 2020-03-02 9:17 ` Stanley Chu 2020-03-02 9:17 ` [f2fs-dev] " Stanley Chu 2020-02-21 11:50 ` [PATCH v7 7/9] fscrypt: add inline encryption support Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 18:40 ` Eric Biggers [this message] 2020-02-21 18:40 ` Eric Biggers 2020-02-22 5:39 ` Eric Biggers 2020-02-22 5:39 ` [f2fs-dev] " Eric Biggers 2020-02-26 0:30 ` Eric Biggers 2020-02-26 0:30 ` [f2fs-dev] " Eric Biggers 2020-02-21 11:50 ` [PATCH v7 8/9] f2fs: " Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 11:50 ` [PATCH v7 9/9] ext4: " Satya Tangirala 2020-02-21 11:50 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-22 5:21 ` Eric Biggers 2020-02-22 5:21 ` [f2fs-dev] " Eric Biggers 2020-02-21 17:16 ` [PATCH v7 0/9] Inline Encryption Support Eric Biggers 2020-02-21 17:16 ` [f2fs-dev] " Eric Biggers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200221184009.GD925@sol.localdomain \ --to=ebiggers@kernel.org \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=satyat@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.