Re: [PATCH v2 13/17] xen/hypfs: support dynamic hypfs nodes

From: "Jürgen Groß" <jgross@suse.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 13/17] xen/hypfs: support dynamic hypfs nodes
Date: Thu, 3 Dec 2020 16:18:20 +0100	[thread overview]
Message-ID: <0efe5d36-da32-7f0d-5515-5fb5994ea2d9@suse.com> (raw)
In-Reply-To: <a02fe2e6-428f-9bea-0108-92fa03729420@suse.com>

[-- Attachment #1.1.1: Type: text/plain, Size: 2250 bytes --]

On 03.12.20 16:08, Jan Beulich wrote:
> On 01.12.2020 09:21, Juergen Gross wrote:
>> Add a HYPFS_VARDIR_INIT() macro for initializing such a directory
>> statically, taking a struct hypfs_funcs pointer as parameter additional
>> to those of HYPFS_DIR_INIT().
>>
>> Modify HYPFS_VARSIZE_INIT() to take the function vector pointer as an
>> additional parameter as this will be needed for dynamical entries.
>>
>> For being able to let the generic hypfs coding continue to work on
>> normal struct hypfs_entry entities even for dynamical nodes add some
>> infrastructure for allocating a working area for the current hypfs
>> request in order to store needed information for traversing the tree.
>> This area is anchored in a percpu pointer and can be retrieved by any
>> level of the dynamic entries. The normal way to handle allocation and
>> freeing is to allocate the data in the enter() callback of a node and
>> to free it in the related exit() callback.
>>
>> Add a hypfs_add_dyndir() function for adding a dynamic directory
>> template to the tree, which is needed for having the correct reference
>> to its position in hypfs.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - switch to xzalloc_bytes() in hypfs_alloc_dyndata() (Jan Beulich)
>> - carved out from previous patch
>> - use enter() and exit() callbacks for allocating and freeing
>>    dyndata memory
> 
> I can't seem to be able to spot what this describes, and the
> respective part of the description therefore also remains unclear

I think all pieces are coming together with patch 15.

> to me. Not the least again when considering multi-level templates,
> where potentially each of the handlers may want to allocate dyndata,
> yet only one party can at a time.

Right now: yes.

In case needed it will be rather easy to have a linked list of dyndata
entities, with the percpu dyndata variable pointing to the most recent
one (the one of the currently deepest nesting level).

> 
>> - add hypfs_add_dyndir()
> 
> Overall this patch adds a lot of (for now) dead code, which makes it
> hard to judge whether this is what's needed. I guess I'll again
> learn more by reding further patches.

I hope so.

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]