From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============6504798098590137253==" MIME-Version: 1.0 From: Harris, James R Subject: Re: [SPDK] spdk_bdev_close() threading Date: Thu, 28 Mar 2019 14:41:30 +0000 Message-ID: <88118D93-4D1E-4404-B44A-47260D271D5B@intel.com> In-Reply-To: 7d98fb2b.2a438.169c3cfe51c.Coremail.wuzhouhui14@mails.ucas.ac.cn List-ID: To: spdk@lists.01.org --===============6504798098590137253== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable =EF=BB=BFOn 3/28/19, 3:20 AM, "SPDK on behalf of wuzhouhui" wrote: = > -----Original Messages----- > From: "Stojaczyk, Dariusz" > Sent Time: 2019-03-28 17:13:16 (Thursday) > To: "Storage Performance Development Kit" > Cc: = > Subject: [SPDK] spdk_bdev_close() threading > = > I was surprised to see that bdev descriptors can be closed only from = the same thread that opened them. Vhost doesn't respect this rule. As expec= ted, I was able to trigger assert(desc->thread =3D=3D spdk_get_thread()) wh= ile closing a vhost scsi descriptor using the latest SPDK master. This coul= d be probably fixed by always scheduling the spdk_bdev_close() to proper th= read. Maybe vhost could even immediately assume the descriptor is closed an= d set its `desc` pointer to NULL without waiting for spdk_bdev_close() to b= e actually called. But why the descriptor needs to be closed from a specifi= c 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 loo= ks good at the first sight for me. I agree we can just have it internally schedule itself. I think we'll stil= l need to set desc->closed =3D true in the caller's thread through before w= e schedule it - otherwise we'll have a race with the _remove_notify path. -Jim = > A descriptor cannot be closed until all associated channel have been = destroyed - that's what the bdev programming guide says. When there are mul= tiple 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 finish= ed. So closing a descriptor will likely have the following flow: > = > external request (e.g. bdev hotremove or some RPC) -> start draining = I/O on all threads -> destroy each i/o channel after its pending i/o has fi= nished -> on the last thread to destroy a channel schedule closing the desc= to a proper thread -> close the desc > = > This additional scheduling of spdk_bdev_close() looks completely unne= cessary - 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 its= elf on desc->thread, so that spdk_bdev_close() can be called from any threa= d? > = > 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 = --===============6504798098590137253==--