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: Tue, 13 Aug 2019 13:57:39 -0400 [thread overview] Message-ID: <CAN-5tyFpHHsH3n8u+qGyp7POdSRHesiKgd3-YQoE9jJSPBVYRw@mail.gmail.com> (raw) In-Reply-To: <20190812200019.GB29812@parsley.fieldses.org> 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? > > --b.
next prev parent reply other threads:[~2019-08-13 17:57 UTC|newest] 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 [this message] 2019-08-14 15:05 ` Olga Kornievskaia 2019-08-29 19:23 ` Olga Kornievskaia 2019-08-30 17:56 ` J. Bruce Fields 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 J. Bruce Fields 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 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=CAN-5tyFpHHsH3n8u+qGyp7POdSRHesiKgd3-YQoE9jJSPBVYRw@mail.gmail.com \ --to=olga.kornievskaia@gmail.com \ --cc=bfields@redhat.com \ --cc=linux-nfs@vger.kernel.org \ --subject='Re: [PATCH v5 5/9] NFSD add COPY_NOTIFY operation' \ /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).