Re: libpager

[Brent W. Baccala]

On Sun, Aug 6, 2017 at 2:34 PM, Justus Winter <justus@gnupg.org> wrote:

"Brent W. Baccala" <cosine@freesoft.org> writes:

>> Are you aware of the pager rework in [0]?  I'm considering to review and
>> merge these changes.  It may make sense to try to come up with a common
>> API.  What is the state of your library, is it a drop-in replacement?
>>
>> 0: https://git.sceen.net/hurd/hurd.git/log/?h=mplaneta/gsoc12/review_v1
>
>
> I was not aware of this link, exactly.  Richard Braun mentioned a GSoC 12
> project a week ago, but I thought that it was a gnumach modification.
> Glancing at this git repository, I see that it's all translator stuff.

Not merely, see e.g.:

https://git.sceen.net/hurd/hurd.git/commit/?h=mplaneta/gsoc12/review_v1&id=2ca8719606eeca99c425b3282017a6412b49213a

OK, so it looks like Neal Walfield, in 2002, modified libpager to pass in pointers to its callback functions instead of linking to fixed names in the translators.  The new callbacks allow for multi-page operations and take block number arguments instead of byte offsets.  These patches were applied into the gsoc12 review branch in 2012 but haven't made it into master.  Not much changed in the libpager logic itself, except that multi-page operations are supported, even though the kernel still doesn't use them without further patches.

Other than passing a structure of pointers to pager_create(), the biggest difference that I see is how the data is returned.  You don't just malloc an array and return it from pager_read_page().  In the new API, the read_pages() function must return the data by calling pager_data_supply(), pager_data_unavailable(), or pager_data_read_error().

The immediate issue that I see with this is that in a multi-client libpager, you can't just wrap pager_data_supply() around memory_object_data_supply() because you don't know which clients to supply the data to.  That means that pager_data_supply() will have to look at the pagemap to figure exactly what to do with the data it's passed.  I don't foresee any serious problems, though.

Obviously, changes are required to the filesystem translators to support the new API.  Some of the translators, like fatfs, were just modified to run a loop over their old, single page code.  That could even be moved to the libpager library; i.e, the structure of function pointers could include both read_page() and read_pages(), and use the old read_page() interface if read_pages() is NULL.

Perhaps I could design libpager to use the new read_pages() interface, but continue supporting the old API for now.  Then we can easily transition to the new interface, perhaps just by changing pager_create() calls to pager_create_ops(), which would take an extra pointer to a struct pager_ops.

What do you guys think?  Do we like Neal Walfield's API?  Start moving in that direction?

    agape

    brent