All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: Richard Weinberger <richard@nod.at>
Cc: David Gstir <david@sigma-star.at>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel@vger.kernel.org, jaegeuk@kernel.org, tytso@mit.edu,
	hch@infradead.org, arnd@arndb.de, dedekind1@gmail.com,
	linux-mtd@lists.infradead.org, adrian.hunter@intel.com,
	linux-ext4@vger.kernel.org, ebiggers@google.com,
	rdunlap@infradead.org
Subject: Re: [PATCH] fscrypt: Factor out bio specific functions
Date: Fri, 16 Dec 2016 14:14:28 -0800	[thread overview]
Message-ID: <20161216221428.GC14264@gmail.com> (raw)
In-Reply-To: <48d4de8e-6e57-cbad-7556-03aa788c815a@nod.at>

On Fri, Dec 16, 2016 at 09:48:19PM +0100, Richard Weinberger wrote:
> On 16.12.2016 16:37, David Gstir wrote:
> >> @@ -349,33 +347,10 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
> >> 		err = do_page_crypto(inode, FS_ENCRYPT, lblk,
> >> 					ZERO_PAGE(0), ciphertext_page,
> >> 					PAGE_SIZE, 0, GFP_NOFS);
> >> +		err = fscrypt_bio_submit_page(inode, pblk, ciphertext_page);
> > 
> > Any specific reason why you didn't just move the whole fscrypt_zeroout_range() to bio.c?
> 
> The function depends other internal functions of crypto.c which I didn't want to
> export.
> At the end of the day it's a matter of taste. I found it less ugly to keep
> fscrypt_zeroout_range() in crypto.c than exposing internal stuff.
> 

Hmm, it still seems weird to define fscrypt_zeroout_range() when it can't
actually be used.  It looks like the problem is specifically the use of
alloc_bounce_page() and do_page_crypto().  Would it be that bad to make those
available in fscrypt_internal.h (not exported to filesystems)?

Also, it seems the actual problem is that fscrypt_zeroout_range() tries to be
clever and reuse one bounce page over and over.  But this seems very inefficient
because it has to wait for each block to be synchronously written out before
moving on to the next.  I'm thinking it really should be updated to work more
like the normal write path, and then it could use fscrypt_encrypt_page()...

Eric

  reply	other threads:[~2016-12-16 22:14 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16 10:50 [PATCH] fscrypt: Factor out bio specific functions Richard Weinberger
2016-12-16 15:37 ` David Gstir
2016-12-16 15:37   ` David Gstir
2016-12-16 20:48   ` Richard Weinberger
2016-12-16 22:14     ` Eric Biggers [this message]
2016-12-16 22:18       ` Richard Weinberger
2016-12-19 10:39       ` Christoph Hellwig
2016-12-19 11:25         ` [PATCH v2] " Richard Weinberger
2016-12-19 22:40           ` Eric Biggers
2016-12-20  5:56           ` Christoph Hellwig
2016-12-20  6:42           ` David Gstir
2017-01-01 21:47           ` Theodore Ts'o
2017-01-03  9:49             ` Richard Weinberger
2017-01-03 14:28               ` Theodore Ts'o
2017-01-04 20:10                 ` Eric Biggers
2017-01-04 22:52                   ` Richard Weinberger
2017-01-07 19:24                   ` Theodore Ts'o
2017-01-07 22:40                     ` Richard Weinberger
2017-01-09 13:33                       ` Christoph Hellwig
2017-01-09 13:33                         ` Christoph Hellwig

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=20161216221428.GC14264@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=adrian.hunter@intel.com \
    --cc=arnd@arndb.de \
    --cc=david@sigma-star.at \
    --cc=dedekind1@gmail.com \
    --cc=ebiggers@google.com \
    --cc=hch@infradead.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=richard@nod.at \
    --cc=tytso@mit.edu \
    /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: link
Be 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.