From: Olga Kornievskaia <olga.kornievskaia@gmail.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: "J. Bruce Fields" <bfields@redhat.com>,
linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v4 4/8] NFSD add COPY_NOTIFY operation
Date: Tue, 30 Jul 2019 12:13:46 -0400 [thread overview]
Message-ID: <CAN-5tyGwyasodrUe4Y+p_Er6XNOBk+mgiaXKXWSBsM5ac4areg@mail.gmail.com> (raw)
In-Reply-To: <20190730155528.GE31707@fieldses.org>
On Tue, Jul 30, 2019 at 11:55 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Tue, Jul 30, 2019 at 11:48:27AM -0400, Olga Kornievskaia wrote:
> > On Tue, Jul 23, 2019 at 4:46 PM J. Bruce Fields <bfields@fieldses.org> wrote:
> > >
> > > On Mon, Jul 22, 2019 at 04:17:44PM -0400, Olga Kornievskaia wrote:
> > > > Let me see if I understand your suspicion and ask for guidance how to
> > > > resolve it as perhaps I'm misusing the function. idr_alloc_cyclic()
> > > > keeps track of the structure of the 2nd arguments with a value it
> > > > returns. How do I initiate the structure with the value of the
> > > > function without knowing the value which can only be returned when I
> > > > call the function to add it to the list? what you are suggesting is to
> > > > somehow get the value for the new_id but not associate anything then
> > > > update the copy structure with that value and then call
> > > > idr_alloc_cyclic() (or something else) to create that association of
> > > > the new_id and the structure? I don't know how to do that.
> > >
> > > You could move the initialization under the s2s_cp_lock. But there's
> > > additional initialization that's done in the caller.
> >
> > I still don't understand what you are looking for here and why. I'm
> > following what the normal stid allocation does. There is no extra code
> > there to see if it initiated or not. nfs4_alloc_stid() calls
> > idr_alloc_cyclic() creates an association between the stid pointer and
> > at the time uninitialized nfs4_stid structure which is then filled in
> > with the return of the idr_alloc_cyclic(). That's exactly what the new
> > code is doing (well accept that i'll change it to store the
> > stateid_t).
>
> Yes, I'm a little worried about normal stid allocation too. It's got
> one extra safeguard because of the check for 0 sc_type in the lookup,
> I haven't yet convinced myself that's enough.
>
> The race I'm worried about is: one task does the idr allocation and
> drops locks. Before it has the chance to finish initializing the
> object, a second task looks it up in the idr and does something with it.
> It sees the not-yet-initialized fields.
Can the spin_lock() that we call before the idr_alloc_cyclic() be held
thru the initialization of the stid then? I'm just not sure what this
idr_preload_end() with a spin_lock but otherwise I don't see why we
can't and since idr_find() takes the same spin lock before the call,
it would solve the problem.
>
> --b.
next prev parent reply other threads:[~2019-07-30 16:13 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-08 19:23 [PATCH v4 0/8] server-side support for "inter" SSC copy Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 1/8] NFSD fill-in netloc4 structure Olga Kornievskaia
2019-07-17 21:13 ` J. Bruce Fields
2019-07-22 19:59 ` Olga Kornievskaia
2019-07-30 15:48 ` Olga Kornievskaia
2019-07-30 15:51 ` J. Bruce Fields
2019-07-08 19:23 ` [PATCH v4 2/8] NFSD add ca_source_server<> to COPY Olga Kornievskaia
2019-07-17 21:40 ` J. Bruce Fields
2019-07-22 20:00 ` Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 3/8] NFSD return nfs4_stid in nfs4_preprocess_stateid_op Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 4/8] NFSD add COPY_NOTIFY operation Olga Kornievskaia
2019-07-09 12:34 ` Anna Schumaker
2019-07-09 15:51 ` Olga Kornievskaia
2019-07-17 22:12 ` J. Bruce Fields
2019-07-17 22:15 ` J. Bruce Fields
2019-07-22 20:03 ` Olga Kornievskaia
2019-07-17 23:07 ` J. Bruce Fields
2019-07-22 20:17 ` Olga Kornievskaia
2019-07-23 20:45 ` J. Bruce Fields
2019-07-30 15:48 ` Olga Kornievskaia
2019-07-30 15:55 ` J. Bruce Fields
2019-07-30 16:13 ` Olga Kornievskaia [this message]
2019-07-30 17:10 ` Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 5/8] NFSD check stateids against copy stateids Olga Kornievskaia
2019-07-19 22:01 ` J. Bruce Fields
2019-07-22 20:24 ` Olga Kornievskaia
2019-07-23 20:58 ` J. Bruce Fields
2019-07-30 16:03 ` Olga Kornievskaia
2019-07-31 21:10 ` Olga Kornievskaia
2019-07-31 21:51 ` J. Bruce Fields
2019-08-01 14:12 ` Olga Kornievskaia
2019-08-01 15:12 ` J. Bruce Fields
2019-08-01 15:41 ` Olga Kornievskaia
2019-08-01 18:06 ` Olga Kornievskaia
2019-08-01 18:11 ` J. Bruce Fields
2019-08-01 18:24 ` Olga Kornievskaia
2019-08-01 19:36 ` J. Bruce Fields
2019-08-07 16:02 ` Olga Kornievskaia
2019-08-07 16:08 ` J. Bruce Fields
2019-08-07 16:42 ` Olga Kornievskaia
2019-08-08 11:25 ` J. Bruce Fields
2019-07-08 19:23 ` [PATCH v4 6/8] NFSD generalize nfsd4_compound_state flag names Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 7/8] NFSD: allow inter server COPY to have a STALE source server fh Olga Kornievskaia
2019-07-23 21:35 ` J. Bruce Fields
2019-07-30 15:48 ` Olga Kornievskaia
2019-07-08 19:23 ` [PATCH v4 8/8] NFSD add nfs4 inter ssc to nfsd4_copy Olga Kornievskaia
2019-07-09 12:43 ` Anna Schumaker
2019-07-09 15:53 ` Olga Kornievskaia
2019-07-09 3:53 ` [PATCH v4 0/8] server-side support for "inter" SSC copy J. Bruce Fields
2019-07-09 15:47 ` Olga Kornievskaia
2019-07-17 18:05 ` Olga Kornievskaia
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-5tyGwyasodrUe4Y+p_Er6XNOBk+mgiaXKXWSBsM5ac4areg@mail.gmail.com \
--to=olga.kornievskaia@gmail.com \
--cc=bfields@fieldses.org \
--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
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).