Bug 26128

Summary: [APPLE_object_purgeable] Purgeable(VOLATILE) should always return VOLATILE
Product: Mesa Reporter: Shuang He <shuang.he>
Component: Drivers/DRI/i965Assignee: Chris Wilson <chris>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: git   
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: piglit case

Description Shuang He 2010-01-19 19:13:48 UTC
Created attachment 32735 [details]
piglit case

System Environment:
--------------------------
mesa (object-purgeable)a5a74202661423326184151e6ba746c3b4564186
(git://people.freedesktop.org/~ickle/mesa)
Libdrm:         (master)db50f5127421ac8f4e3ce4eb7c27d27475781488
Xserver:               
(server-1.7-branch)aea5ace1ee331fab0b72885ce0d5d3fc235e0708
Xf86_video_intel:              
(master)8ecf70ea553083cbc26928dc3973c8f6f8b3d9d0



Bug detailed description:
-------------------------
I'm testing this extension on my G41.
According to the spec of GL_APPLE_object_purgeable, 
"If ObjectPurgeableAPPLE is called with an <option> of
    VOLATILE_APPLE, then ObjectPurgeableAPPLE will also return the value
    VOLATILE_APPLE."

Purgeable(VOLATILE) should always return VOLATILE

You could run and check attached pigit case
Comment 1 Chris Wilson 2010-03-05 02:29:37 UTC
The spec is self-contradictory here:

"If ObjectPurgeableAPPLE is called with an  of
 VOLATILE_APPLE, then ObjectPurgeableAPPLE will also return the value
 VOLATILE_APPLE."

followed by:

"If ObjectPurgeableAPPLE returns the value VOLATILE_APPLE, this means
 that the storage for the object may not have been released when
 ObjectPurgeableAPPLE returns.  In contrast, if ObjectPurgeableAPPLE
 returns the value RELEASED_APPLE, this means that the storage for
 the object has been released when ObjectPurgeableAPPLE returns."

The current behaviour follows the second paragraph, reporting the known state of the underlying buffer after marking it purgeable.

Comment 2 Shuang He 2010-03-05 06:34:23 UTC
(In reply to comment #1)
> The spec is self-contradictory here:
> 
> "If ObjectPurgeableAPPLE is called with an  of
>  VOLATILE_APPLE, then ObjectPurgeableAPPLE will also return the value
>  VOLATILE_APPLE."
> 
> followed by:
> 
> "If ObjectPurgeableAPPLE returns the value VOLATILE_APPLE, this means
>  that the storage for the object may not have been released when
>  ObjectPurgeableAPPLE returns.  In contrast, if ObjectPurgeableAPPLE
>  returns the value RELEASED_APPLE, this means that the storage for
>  the object has been released when ObjectPurgeableAPPLE returns."
> 
> The current behaviour follows the second paragraph, reporting the known state
> of the underlying buffer after marking it purgeable.
> 


I think those paragraphs should be taken this way:
    if (call ObjectPurgeableAPPLE with VOLATILE_APPLE) {
        it must return VOLATILE_APPLE
    } 
    else if (call ObjectPurgeableAPPLE with VOLATILE_APPLE) {
        if (ObjectPurgeableAPPLE returns VOLATILE_APPLE) {
            "may not have been released"
        }
        else if (ObjectPurgeableAPPLE returns RELEASED_APPLE) {
            "have been released"
        }
    }
Then it won't conflict itself.



        

Comment 3 Chris Wilson 2010-03-05 09:23:06 UTC
I believe the mesa side of the code is correct, all it is doing is reporting the status of the object as reported by the kernel. To misrepresent that information as suggested by the first paragraph is nonsense and in conflict with the second.

What the kernel is doing is actually evicting an inactive buffer on the madvise(DONTNEED), this may not be the desired behaviour and is causing the conflict with the spec. (Though there still may be some tight memory conditions where immediate eviction may happen, so RELEASED could still be reported; i.e. VOLATILE can not always be guaranteed - it is simply outside the control of mesa.)
Comment 4 Shuang He 2010-03-05 16:45:55 UTC
If I understand the spec correctly.
Once you called ObjectPurgeableAPPLE, the object is in the dying edge. You have to call ObjectUnpurgeableAPPLE to make sure the object is alive.
Everything return from objectPurgeableAPPLE is just for-your-information
Comment 5 Chris Wilson 2010-03-06 00:17:00 UTC
Yes, that is essentially the opinion of the spec authors as well:

"ObjectPurgeable(VOLATILE) must return VOLATILE because there is no other return value that is useful to the application.  I do understand that your implementation may actually know that the resource will be released, therefore a following call to ObjectUnpurgeable(RETAINED) will, with no variability, return UNDEFINED.  That's also true with other OS's - and that's fine.  The spec was written with this in mind.  ObjectPurgeable returns VOLATILE, and the app must call ObjectUnpurgeable(RETAINED) to find out that the storage was actually released.  It's OK that the implementation actually knew that the object was released sooner - ObjectPurgeable and ObjectUnpurgeable calls must still be matched."

Given the "there is no other useful return value", why bother returning anything? Gah.

Anyway, Ian asked for us to conform:

commit 24f90112761d108a4a131fad11bd7b426d8edfa0
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Mar 5 23:10:45 2010 +0000

    Always return VOLATILE for ObjectPurgeable(VOLATILE)
    
    Fixes fdo bug 26128.
    
    The spec mandates that VOLATILE is returned from
    ObjectPurgeable(VOLATILE) irrespective of the actual status of the
    object upon completion of marking it purgeable.
    
    Conform to the spec, even though it seems wrong.
    
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I'll update the comments to reflect the rationale later.
Comment 6 Shuang He 2010-03-10 18:22:32 UTC
Thanks, I'd go ahead to verify this bug first. It's working with:
Libdrm:         (master)04fd3872ee8bd8d5e2c27740508c67c2d51dbc11
Mesa:           (master)6f4ce4a4fed9f0f0f0ee89a63e406ab86dae7150
Xserver:                (master)bbae92795c7eab062e6722c42fa7915e0cee5d69
Xf86_video_intel:              
(master)318aa9ed799197810e2039dbe3ec51559dcc888c
Kernel:       (drm-intel-next)338d762fc2dc2c1493813123fc4cea998bb3e683

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.