Re: [PATCH] mach_defpager: Fix external objects interface

[Samuel Thibault]

Sergio Lopez, le Wed 19 May 2010 16:29:21 +0200, a écrit :
> OK, here we go:

Much easier to review, thanks.

>   - 01defpager-mutex.patch: Properly unlock the mutex before returning
>     NO_BLOCK in pager_read_offset.

Applied.

>   - 02defpager-extread.patch: Add an "external" attribute to
>     dstruct structure. In default_read, if the object is external,
>     resolve a read request with a zero filled page.

Applied.

>   - 03defpager-synclock.patch: Remove user reference count logic, to
>     avoid setting an arbitrary limit to the number of requests
>     processed by an object. Don't request completion notification for
>     m_o_lock_request.

Are these two parts related?

For the first part, I'm not really at ease with applying it, are these
requests potentially from any user process?  If so, we're opening yet
another breach to the user, aren't we?

As for the second part, what is the motivation?  What is very noticeable
is that pager_truncate() doesn't get called any more with your patch :).
This needs further explanations.

>     I think is worth noting that the original problem
>     which makes default_pager get stuck waiting for a seqno that never
>     arrives in S_default_pager_object_set_size, was just that
>     reply_port and seqno arguments were in wrong order.

D'oh!  At least in the meanwhile I have commited a fix for that.

>   - 04defpager-objectsize.patch: In S_default_pager_object_create,
>     reject values for "size" other than "vm_page_size", since internal
>     logic doesn't properly support it (though object size limit can be
>     increased by sucesive calls to S_default_pager_set_size).

Mmm. Which logic doesn't properly support it?  There at least is some
code to handle != vm_page_size.  Isn't it possible to fix potential bugs
there?  I'm not at ease with this change because there are known clients
(tmpfs and boot) which won't be able to cope with that.

Samuel