linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Shilovsky <piastryyy@gmail.com>
To: Shyam Prasad N <nspmangalore@gmail.com>
Cc: ronnie sahlberg <ronniesahlberg@gmail.com>,
	Steve French <smfrench@gmail.com>,
	CIFS <linux-cifs@vger.kernel.org>
Subject: Re: cifs.ko page management during reads
Date: Wed, 14 Jul 2021 09:34:30 -0700	[thread overview]
Message-ID: <CAKywueRVSWVtD3KRcOTCAGCsxuoL_XUgm9Ga1eTs0K53Z47HDg@mail.gmail.com> (raw)
In-Reply-To: <CANT5p=oCJ5MRSVcNHV3-+Jg2TcGc3m785GGJBzScGThkS24eMw@mail.gmail.com>

вт, 13 июл. 2021 г. в 23:51, Shyam Prasad N <nspmangalore@gmail.com>:
>
> On Wed, Jul 14, 2021 at 5:45 AM ronnie sahlberg
> <ronniesahlberg@gmail.com> wrote:
> >
> > On Wed, Jul 14, 2021 at 8:58 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
> > >
> > > пн, 12 июл. 2021 г. в 22:41,Steve French <smfrench@gmail.com>:
> > > >
> > > > And don't forget "esize" mount option to allow offload of decryption...
> > > >
> > > > On Tue, Jul 13, 2021, 00:37 Shyam Prasad N <nspmangalore@gmail.com> wrote:
> > > >>
> > > >> Hi all,
> > > >>
> > > >> I'm trying to understand our read path (big picture is to integrate
> > > >> the new netfs helpers into cifs.ko), and I wanted to get some
> > > >> understanding about why things are the way they are. @Pavel Shilovsky
> > > >> I'm guessing that you've worked on most of this code, so hoping that
> > > >> you'll be able to answer some of these:
> > >
> > > Thanks for taking a look at this.
> > >
> > > >>
> > > >> 1. for cases where both encryption and signing are disabled, it looks
> > > >> like we read from the socket page-by-page directly into pages and map
> > > >> those to the inode address space
> > >
> > > Yes, we read directly into pre-allocated pages. For direct IO we read
> > > into pages supplied by the user and for non-direct IO (including page
> > > IO) we allocate intermediate pages.
> > >
> > > >>
> > > >> 2. for cases where encryption is enabled, we read the encrypted data
> > > >> into a set of pages first, then call crypto APIs, which decrypt the
> > > >> data in-place to the same pages. We then copy out the decrypted data
> > > >> from these pages into the pages in the address space that are already
> > > >> mapped.
> > >
> > > Correct. Regardless of whether offloading is used or not there is an extra copy.
> > >
> > > >>
> > > >> 3. similarly for the signing case, we read the data into a set of
> > > >> pages first, then verify signature, then copy the pages to the mapped
> > > >> pages in the inode address space.
> > >
> > > Yes.
> > >
> > > >>
> > > >> for case 1, can't we read from the socket directly into the mapped
> > > >> pages?
> > >
> > > Yes - assuming no signing or encryption, we should be able to read
> > > directly into the mapped pages. But I doubt many people use SMB2
> > > without signing or encryption although there may be some use cases
> > > requiring performance over safety.
> > >
> > > >> for case 2 and 3, instead of allocating temporary pages to read
> > > >> into, can't we read directly into the pages of the inode address
> > > >> space? the only risk I see is that if decryption or sign verification
> > > >> fails, it would overwrite the data that is already present in the page
> > > >> cache. but I see that as acceptable, because we're reading from the
> > > >> server, since the data we have in the cache is stale anyways.
> > >
> > > Theoretically we may try doing this with signing but we need to make
> > > sure that no one can modify those pages locally which may lead to
> > > signing errors. I don't think today we reconnect an SMB session on
> > > those today but we probably should.
> > >
>
> How can someone modify those pages? Don't we keep the pages locked
> when the read is in progress?
>

Agree. As I said - theoretically we can do such an optimization. I am
not sure if it is worth it given the whole signing overhead.

--
Best regards,
Pavel Shilovsky

      reply	other threads:[~2021-07-14 16:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13  5:37 Shyam Prasad N
     [not found] ` <CAH2r5mvxj-A3F1tPr9OH1D00bdznVYyx7FzyjLZt=Xq41TCVxw@mail.gmail.com>
2021-07-13 22:58   ` Pavel Shilovsky
2021-07-14  0:15     ` ronnie sahlberg
2021-07-14  6:50       ` Shyam Prasad N
2021-07-14 16:34         ` Pavel Shilovsky [this message]

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=CAKywueRVSWVtD3KRcOTCAGCsxuoL_XUgm9Ga1eTs0K53Z47HDg@mail.gmail.com \
    --to=piastryyy@gmail.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=nspmangalore@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=smfrench@gmail.com \
    --subject='Re: cifs.ko page management during reads' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).