On 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 from the same thread that opened them. Vhost doesn't respect this rule. As expected, I was able to trigger assert(desc->thread == spdk_get_thread()) 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 specific 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 looks good at the first sight for me.

I agree we can just have it internally schedule itself.  I think we'll still need to set desc->closed = true in the caller's thread through before we 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 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 draining I/O on all threads -> destroy each i/o channel after its pending i/o has finished -> 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 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 itself 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