From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============0095201711381929443=="
MIME-Version: 1.0
From: Stojaczyk, Dariusz <dariusz.stojaczyk at intel.com>
Subject: Re: [SPDK] spdk_bdev_close() threading
Date: Thu, 28 Mar 2019 14:56:39 +0000
Message-ID: <FBE7E039FA50BF47A673AD0BD3CD56A8462F2F66@HASMSX105.ger.corp.intel.com>
In-Reply-To: 88118D93-4D1E-4404-B44A-47260D271D5B@intel.com
List-ID: <spdk@lists.01.org>
To: spdk@lists.01.org

--===============0095201711381929443==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable



> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Harris, Jame=
s R
> Sent: Thursday, March 28, 2019 3:42 PM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] spdk_bdev_close() threading
> =

> =

> =

> =EF=BB=BFOn 3/28/19, 3:20 AM, "SPDK on behalf of wuzhouhui" <spdk-
> bounces(a)lists.01.org on behalf of wuzhouhui14(a)mails.ucas.ac.cn> wrote:
> =

> =

>     > -----Original Messages-----
>     > From: "Stojaczyk, Dariusz" <dariusz.stojaczyk(a)intel.com>
>     > Sent Time: 2019-03-28 17:13:16 (Thursday)
>     > To: "Storage Performance Development Kit" <spdk(a)lists.01.org>
>     > Cc:
>     > Subject: [SPDK] spdk_bdev_close() threading
>     >
>     > I was surprised to see that bdev descriptors can be closed only fro=
m the
> same thread that opened them. Vhost doesn't respect this rule. As
> expected, I was able to trigger assert(desc->thread =3D=3D spdk_get_threa=
d())
> while closing a vhost scsi descriptor using the latest SPDK master. This =
could
> be probably fixed by always scheduling the spdk_bdev_close() to proper
> thread. Maybe vhost could even immediately assume the descriptor is closed
> and set its `desc` pointer to NULL without waiting for spdk_bdev_close() =
to
> be actually called. But why the descriptor needs to be closed from a spec=
ific
> thread in the first place? Would it be possible for spdk_bdev_close() to
> internally schedule itself on desc->thread?
>     >
>     Darek:
> =

>     The purpose spdk_bdev_close() running on the same thread with
> spdk_bdev_open() is a preparation of patch
> https://review.gerrithub.io/c/spdk/spdk/+/423369.
> =

>     Let spdk_bdev_close() to internally schedule itself on desc->thread l=
ooks
> good at the first sight for me.
> =

> I agree we can just have it internally schedule itself.  I think we'll st=
ill need to
> set desc->closed =3D true in the caller's thread through before we schedu=
le it -
> otherwise we'll have a race with the _remove_notify path.

Okay, cool. Do we have any unit tests for that?

D.

> =

> -Jim
> =

> =

>     > A descriptor cannot be closed until all associated channel have been
> destroyed - that's what the bdev programming guide says. When there are
> multiple I/O channels, there has to be some multi-thread management
> involved. Also, those channels can't be closed until all their pending I/=
O has
> finished. So closing a descriptor will likely have the following flow:
>     >
>     > external request (e.g. bdev hotremove or some RPC) -> start drainin=
g I/O
> on all threads -> destroy each i/o channel after its pending i/o has fini=
shed ->
> on the last thread to destroy a channel schedule closing the desc to a pr=
oper
> thread -> close the desc
>     >
>     > This additional scheduling of spdk_bdev_close() looks completely
> unnecessary - it also forces the upper layer to maintain a pointer to the=
 desc
> thread somewhere, because desc->thread is private to the bdev layer. So to
> repeat the question again -
>     > would it be possible for spdk_bdev_close() to internally schedule i=
tself
> on desc->thread, so that spdk_bdev_close() can be called from any thread?
>     >
>     > D.
>     >
>     > _______________________________________________
>     > SPDK mailing list
>     > SPDK(a)lists.01.org
>     > https://lists.01.org/mailman/listinfo/spdk
>     _______________________________________________
>     SPDK mailing list
>     SPDK(a)lists.01.org
>     https://lists.01.org/mailman/listinfo/spdk
> =

> =

> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

--===============0095201711381929443==--