linux-kernel - Re: [PATCH 1/6] xenbus: prepare data structures and parameter for xenwatch multithreading

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20422e39-f812-554e-f711-186bc6e00005@oracle.com>
Date:   Tue, 25 Sep 2018 16:19:05 -0400
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Dongli Zhang <dongli.zhang@...cle.com>,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Cc:     jgross@...e.com, paul.durrant@...rix.com, wei.liu2@...rix.com,
        konrad.wilk@...cle.com, roger.pau@...rix.com,
        srinivas.eeda@...cle.com
Subject: Re: [PATCH 1/6] xenbus: prepare data structures and parameter for
 xenwatch multithreading

On 9/25/18 1:14 AM, Dongli Zhang wrote:
>
> So far we have: (1) domain hash table, (2) domain list (where duplicate entries
> may exist) and (3) purge list.
>
> Can I assume you would like to discard the domain list and only keep domain hash
> table and purge list?

Yes, that's what I was thinking.

>
> The purpose of the domain list is to facilitate the unregister_xenbus_watch() to
> scan the pending events for all otherend id. Should we remove it? Xen hypervisor
> used both a hash table and a list to manage struct domain.


Your concern is that searching for a pending event in the hash is not
especially efficient. Since you are hashing on domain_id, then
unregister_single_mtwatch() should be pretty fast if searching in the
hash table (in fact, it's faster than traversing the domain list).
unregister_all_mtwatch() will indeed take longer since you will be
checking empty buckets. Right?

How often do you expect will we call unregister_all_mtwatch() compared
to unregister_single_mtwatch()?


>
>
> About the duplicate entries in the domain list, can we change the flow like below:
>
> 1. Suppose the thread status is DOWN. To avoid having duplicate entries on the
> domain list, instead of keeping the deprecated thread on domain list (until all
> its events get processed), we move it to the purge list immediately.
>
> We will tag this deprecated thread so that the purge list would not purge it
> unless all events belong to such thread get processed. We cannot purge it
> immediately because the worker thread (to purge the purge list) would hang if
> the deprecated thread is already stuck.
>
> In this flow, we may have duplicate entries on purge list, but not domain list.
>
> 2. During unregister_xenbus_watch(), we need to scan both the domain list and
> purge list to remove pending events for the watch. In previous design, we only
> scan domain lit.
>
>
> One option is we allow the deprecated thread to resurrect and we would not move
> the thread to purge list immediately when the thread is deprecated.
>
> Suppose when thread for domid=9 is needed, we would not create new one if the
> deprecated one for domid=9 is still in domain list. Instead, we would resurrect
> it, change its status and reuse it again. In this way, we would not have
> duplicate entries on the domain list.
>
> I like the 1st option. I do not like to resurrect a deprecated thread again.
> Would you please let me know how you think about it?


Yes, I also think (1) is better --- you are walking the whole purge list
anyway.


-boris