All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: Olga Kornievskaia <kolga@netapp.com>,
	linux-nfs <linux-nfs@vger.kernel.org>,
	Anna Schumaker <Anna.Schumaker@netapp.com>
Subject: Re: handle_async_copy calling kzalloc under spinlock
Date: Fri, 16 Nov 2018 14:30:16 -0500	[thread overview]
Message-ID: <20181116193016.GA22304@fieldses.org> (raw)
In-Reply-To: <CAN-5tyHX3h6TXJhFeZPGZvAWXXxwaxpAkZtRNV9+L8m5xJ3fVw@mail.gmail.com>

On Fri, Nov 16, 2018 at 01:52:29PM -0500, Olga Kornievskaia wrote:
> On Fri, Nov 16, 2018 at 1:30 PM Olga Kornievskaia <aglo@umich.edu> wrote:
> > Then how does the copy knows not to go wait for the callback? Copy
> > checks the pending_callback list to see if received a callback. If
> > not, it puts itself on the copy list and goes to sleep. The callback,
> > checks the copy list and if it finds a copy signals it, if not it puts
> > itself on the pending_callback list. a lock is held over checking one
> > list and putting yourself on the other.

OK, apologies, I don't really understand those data structures yet, but
something seems wrong to me.

Under what circumstances could we recieve a CB_OFFLOAD without having
started the corresponding copy already?

And shouldn't CB_OFFLOAD be returning bad_stateid in the case it doesn't
recognize the given stateid?  It looks like the allocation failure is
the *only* way we'll return an error on CB_OFFLOAD, and that seems
wrong.

> > > I also wonder if SERVERFAULT is really the best error for a memory
> > > allocation failure there.
> >
> > I guess EIO or ENOMEM might be better. But I don't think this error
> > gets returned anywhere to the main process.
> >
> 
> Wait. It is returning SERVERFAULT because it's the callback server replying
> back to the server's CB_RECALL call and I believe SERVERFAULT is the
> appropriate error here. NFS doesn't have ENOMEM error.

We could return DELAY if we think it might be worth the server trying
the CB_RECALL again.  (That's what nfsd usually returns on allocation
failures.  I don't know if that's really ideal.)

--b.

  parent reply	other threads:[~2018-11-16 19:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-16 14:26 handle_async_copy calling kzalloc under spinlock J. Bruce Fields
     [not found] ` <CAN-5tyGsDAd2hkaw5nONdS2TzRy1qK7xyFDpNeT7Jsd9ZrH7+g@mail.gmail.com>
2018-11-16 17:56   ` J. Bruce Fields
2018-11-16 18:01     ` J. Bruce Fields
2018-11-16 18:30       ` Olga Kornievskaia
     [not found]         ` <CAN-5tyHX3h6TXJhFeZPGZvAWXXxwaxpAkZtRNV9+L8m5xJ3fVw@mail.gmail.com>
2018-11-16 19:30           ` J. Bruce Fields [this message]
2018-11-16 19:49             ` Olga Kornievskaia
2018-11-16 19:58               ` J. Bruce Fields
2018-11-16 20:11                 ` Olga Kornievskaia
2018-11-19 21:05                   ` J. Bruce Fields

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=20181116193016.GA22304@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=Anna.Schumaker@netapp.com \
    --cc=aglo@umich.edu \
    --cc=kolga@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    /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.