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

> "Brent W. Baccala" <cos...@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/gsoc1
> 2/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?


Reply via email to