Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: Olga Kornievskaia <olga.kornievskaia@gmail.com>
To: "J. Bruce Fields" <bfields@redhat.com>
Cc: linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v5 5/9] NFSD add COPY_NOTIFY operation
Date: Wed, 14 Aug 2019 11:05:23 -0400
Message-ID: <CAN-5tyEX=4pfntZudpfkN9Pr9OpZfpvKj+NyT-bBD9YJ5fsg7Q@mail.gmail.com> (raw)
In-Reply-To: <CAN-5tyFpHHsH3n8u+qGyp7POdSRHesiKgd3-YQoE9jJSPBVYRw@mail.gmail.com>

On Tue, Aug 13, 2019 at 1:57 PM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> On Mon, Aug 12, 2019 at 4:00 PM J. Bruce Fields <bfields@redhat.com> wrote:
> >
> > On Mon, Aug 12, 2019 at 03:16:47PM -0400, Olga Kornievskaia wrote:
> > > On Mon, Aug 12, 2019 at 12:19 PM Olga Kornievskaia
> > > <olga.kornievskaia@gmail.com> wrote:
> > > > While this passes my testing, in theory this allows for the race that
> > > > we get the copy notify size but then offload_cancel arrive and change
> > > > the value. Then refcount_sub_and test_check would have an incorrect
> > > > value (can subtract larger than an actual reference count). I have no
> > > > solution for that as there is no refcount_sub_and_lock() that will
> > > > allow to decrement by a multiple under a lock. Thoughts?
> > >
> > > I tried not to use the client's cl_lock but instead use a specific
> > > lock to protect the copy notifies stateid on the stateid list. But
> > > since stateid's reference counter (sc_count) is protected by it, I
> > > think by getting rid of the special lock and using cl_lock will solve
> > > the problem of coordinating access between the sc_count and the
> > > copy_notify stateid list. Are the any problems with using such a big
> > > lock?
> >
> > Probably not.  But it can be confusing when a single lock is used for
> > several different things.  A comment explaining why you need it might
> > help.
>
> While holding the client's cl_lock to manipulate the list of copy
> notify stateids solves the refcount problem. It generates a different
> problem for the laundromat thread. There, client list is traversed
> already holding the cl_lock, so I can't call routines to free
> copy_notify stateid because in turn it calls nfs4_put_stid() which
> wants to take the cl_lock. Putting the copy_notify stateid on the
> reaplist and then I lose a pointer to the client structure that I need
> to take the lock. Then it seems the nfs4_cpntf_state structure would
> need to keep a pointer to the client structure but then I get a
> problem of making sure the nfs4_client structure isn't going away and
> because it even a bigger mess.
>
> I think I need to remove the code in the laundromat that looks for the
> not referenced copy_notifies stateid and just rely on cleaning on the
> removal of the stateid (basically what I originally had). Or I need to
> rely on the client to always send FREE_STATEID. I don't see other
> options, do you?

Ignore this Bruce. Trond gave me a good idea and gets me unstuck.

>
> >
> > --b.

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08 20:18 [PATCH v5 0/9] server-side support for "inter" SSC copy Olga Kornievskaia
2019-08-08 20:18 ` [PATCH v5 1/9] NFSD fill-in netloc4 structure Olga Kornievskaia
2019-08-11  5:48   ` kbuild test robot
2019-08-12 16:12     ` Olga Kornievskaia
2019-08-12 19:58       ` J. Bruce Fields
2019-08-08 20:18 ` [PATCH v5 2/9] NFSD add ca_source_server<> to COPY Olga Kornievskaia
2019-08-11  5:59   ` kbuild test robot
2019-08-11  7:00   ` kbuild test robot
2019-08-08 20:18 ` [PATCH v5 3/9] NFSD return nfs4_stid in nfs4_preprocess_stateid_op Olga Kornievskaia
2019-08-08 20:18 ` [PATCH v5 4/9] NFSD COPY_NOTIFY xdr Olga Kornievskaia
2019-08-11  6:10   ` kbuild test robot
2019-08-08 20:18 ` [PATCH v5 5/9] NFSD add COPY_NOTIFY operation Olga Kornievskaia
2019-08-11  6:17   ` kbuild test robot
2019-08-12 16:19   ` Olga Kornievskaia
2019-08-12 19:16     ` Olga Kornievskaia
2019-08-12 20:00       ` J. Bruce Fields
2019-08-12 20:00         ` J. Bruce Fields
2019-08-13 17:57         ` Olga Kornievskaia
2019-08-14 15:05           ` Olga Kornievskaia [this message]
2019-08-29 19:23             ` Olga Kornievskaia
2019-08-30 17:56               ` bfields
2019-08-08 20:18 ` [PATCH v5 6/9] NFSD check stateids against copy stateids Olga Kornievskaia
2019-08-08 20:18 ` [PATCH v5 7/9] NFSD generalize nfsd4_compound_state flag names Olga Kornievskaia
2019-08-08 20:18 ` [PATCH v5 8/9] NFSD: allow inter server COPY to have a STALE source server fh Olga Kornievskaia
2019-08-08 20:18 ` [PATCH v5 9/9] NFSD add nfs4 inter ssc to nfsd4_copy Olga Kornievskaia
2019-08-11  6:24   ` kbuild test robot
2019-09-04 20:50 ` [PATCH v5 0/9] server-side support for "inter" SSC copy bfields
2019-09-05  0:05   ` Olga Kornievskaia
2019-09-05  0:13     ` Rick Macklem
2019-09-06 14:23       ` Olga Kornievskaia
2019-09-06 15:32         ` Rick Macklem

Reply instructions:

You may reply publically 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='CAN-5tyEX=4pfntZudpfkN9Pr9OpZfpvKj+NyT-bBD9YJ5fsg7Q@mail.gmail.com' \
    --to=olga.kornievskaia@gmail.com \
    --cc=bfields@redhat.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

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org linux-nfs@archiver.kernel.org
	public-inbox-index linux-nfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox