There's neither documentation of what this flag is supposed to mean, nor
any implementation. With this, don't even bother enclosing the #define-s
in a __XEN_INTERFACE_VERSION__ conditional, but drop them altogether.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -628,9 +628,6 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flu
 #define _GNTMAP_contains_pte    (4)
 #define GNTMAP_contains_pte     (1<<_GNTMAP_contains_pte)
 
-#define _GNTMAP_can_fail        (5)
-#define GNTMAP_can_fail         (1<<_GNTMAP_can_fail)
-
 /*
  * Bits to be placed in guest kernel available PTE bits (architecture
  * dependent; only supported when XENFEAT_gnttab_map_avail_bits is set).

On 26/08/2021 11:13, Jan Beulich wrote:
> There's neither documentation of what this flag is supposed to mean, nor
> any implementation. With this, don't even bother enclosing the #define-s
> in a __XEN_INTERFACE_VERSION__ conditional, but drop them altogether.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

It was introduced in 4d45702cf0398 along with GNTST_eagain, and the
commit message hints that it is for xen-paging

Furthermore, the first use of GNTST_eagain was in ecb35ecb79e0 for
trying to map/copy a paged-out frame.

Therefore I think it is reasonable to conclude that there was a planned
interaction between GNTMAP_can_fail and paging, which presumably would
have been "don't pull this in from disk if it is paged out".


I think it is fine to drop, as no implementation has ever existed in
Xen, but it would be helpful to have the history summarised in the
commit message.

~Andrew

On 26.08.2021 13:45, Andrew Cooper wrote:
> On 26/08/2021 11:13, Jan Beulich wrote:
>> There's neither documentation of what this flag is supposed to mean, nor
>> any implementation. With this, don't even bother enclosing the #define-s
>> in a __XEN_INTERFACE_VERSION__ conditional, but drop them altogether.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> It was introduced in 4d45702cf0398 along with GNTST_eagain, and the
> commit message hints that it is for xen-paging
> 
> Furthermore, the first use of GNTST_eagain was in ecb35ecb79e0 for
> trying to map/copy a paged-out frame.
> 
> Therefore I think it is reasonable to conclude that there was a planned
> interaction between GNTMAP_can_fail and paging, which presumably would
> have been "don't pull this in from disk if it is paged out".

I suppose that's (far fetched?) guesswork.

> I think it is fine to drop, as no implementation has ever existed in
> Xen, but it would be helpful to have the history summarised in the
> commit message.

I've extended it to

"There's neither documentation of what this flag is supposed to mean, nor
 any implementation. Commit 4d45702cf0398 ("paging: Updates to public
 grant table header file") suggests there might have been plans to use it
 for interaction with mem-paging, but no such functionality has ever
 materialized. With this, don't even bother enclosing the #define-s in a
 __XEN_INTERFACE_VERSION__ conditional, but drop them altogether."

Jan

On 26/08/2021 14:03, Jan Beulich wrote:
> On 26.08.2021 13:45, Andrew Cooper wrote:
>> On 26/08/2021 11:13, Jan Beulich wrote:
>>> There's neither documentation of what this flag is supposed to mean, nor
>>> any implementation. With this, don't even bother enclosing the #define-s
>>> in a __XEN_INTERFACE_VERSION__ conditional, but drop them altogether.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> It was introduced in 4d45702cf0398 along with GNTST_eagain, and the
>> commit message hints that it is for xen-paging
>>
>> Furthermore, the first use of GNTST_eagain was in ecb35ecb79e0 for
>> trying to map/copy a paged-out frame.
>>
>> Therefore I think it is reasonable to conclude that there was a planned
>> interaction between GNTMAP_can_fail and paging, which presumably would
>> have been "don't pull this in from disk if it is paged out".
> I suppose that's (far fetched?) guesswork.

Not really - the phrase "far fetched" loosely translates as implausible
or unlikely, and I wouldn't characterise the suggestion as implausible.

It is guesswork, and the most plausible guess I can think of.  It is
clear that GNTMAP_can_fail is related to paging somehow, which means
there is some interaction with the gref not being mappable right now.

>
>> I think it is fine to drop, as no implementation has ever existed in
>> Xen, but it would be helpful to have the history summarised in the
>> commit message.
> I've extended it to
>
> "There's neither documentation of what this flag is supposed to mean, nor
>  any implementation. Commit 4d45702cf0398 ("paging: Updates to public
>  grant table header file") suggests there might have been plans to use it
>  for interaction with mem-paging, but no such functionality has ever
>  materialized. With this, don't even bother enclosing the #define-s in a
>  __XEN_INTERFACE_VERSION__ conditional, but drop them altogether."

LGTM.  Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> Jan
>