netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dominique Martinet <dominique.martinet@cea.fr>
To: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Latchesar Ionkov <lucho@ionkov.net>,
	V9FS Developers <v9fs-developer@lists.sourceforge.net>,
	netdev@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Ron Minnich <rminnich@sandia.gov>
Subject: Re: [V9fs-developer] [PATCH] 9p: trans_fd, initialize recv fcall properly if not set
Date: Sun, 6 Sep 2015 08:55:40 +0200	[thread overview]
Message-ID: <20150906065540.GA2121@nautica> (raw)
In-Reply-To: <CAFkjPT=1TAq5S_TBgKa_UyGs2CfhDKOWEqc9HyhKHw89vO_ZOQ@mail.gmail.com>

Eric Van Hensbergen wrote on Sat, Sep 05, 2015:
> On Thu, Sep 3, 2015 at 4:38 AM, Dominique Martinet
> <dominique.martinet@cea.fr> wrote:
> > To be honest, I think it might be better to just bail out if we get in
> > this switch (m->req->rc == NULL after p9_tag_lookup) and not try to
> > allocate more, because if we get there it's likely a race condition and
> > silently re-allocating will end up in more troubles than trying to
> > recover is worth.
> > Thoughts ?
> >
> 
> Hmmm...trying to rattle my brain and remember why I put it in there
> back in 2008.
> It might have just been over-defensive programming -- or more likely it just
> pre-dated all the zero copy infrastructure which pretty much guaranteed we had
> an rc allocated and what is there is vestigial.  I'm happy to accept a
> patch which
> makes this an assert, or perhaps just resets the connection because something
> has gone horribly wrong (similar to the ENOMEM path that is there now).

Yeah, it looks like the safety comes from the zero-copy stuff that came
much later.
Let's go with resetting the connection then. Hmm. EIO is a bit too
generic so would be good to avoid that if possible, but can't think of
anything better...


Speaking of zero-copy, I believe it should be fairly straight-forward to
implement for trans_fd now I've actually looked at it, since we do the
payload read after a p9_tag_lookup, would just need m->req to point to a
zc buffer. Write is similar, if there's a zc buffer just send it after
the header.
The cost is a couple more pointers in req and an extra if in both
workers, that seems pretty reasonable.

Well, I'm not using trans_fd much here (and unfortunately zero-copy
isn't possible at all given the transport protocol for RDMA, at least
for recv), but if anyone cares it probably could be done without too
much hassle for the fd workers.

-- 
Dominique

  reply	other threads:[~2015-09-06  6:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-03  9:38 [PATCH] 9p: trans_fd, initialize recv fcall properly if not set Dominique Martinet
2015-09-06  3:15 ` Eric Van Hensbergen
2015-09-06  6:55   ` Dominique Martinet [this message]
2015-09-07 14:09     ` [V9fs-developer] " Eric Van Hensbergen
2015-09-07 15:06   ` [PATCH v2] 9p: trans_fd, bail out if recv fcall if missing Dominique Martinet

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=20150906065540.GA2121@nautica \
    --to=dominique.martinet@cea.fr \
    --cc=ericvh@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=rminnich@sandia.gov \
    --cc=v9fs-developer@lists.sourceforge.net \
    /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 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).