Linus Torvalds wrote:
On Sat, 22 Feb 2003, Keith Whitwell wrote:

So, was the gist of the fix to simply relocate the current release() stuff to flush()? I'm going to go & read the code now.


Yes, either that, or you need to not care about pid's.

"release()" is not necessarily called _at_all_ within the context of the process that does a close(). Things like /proc/<pid>/fd accesses may mean that it's a process like "ps" that actually does the last release of a file, not the process that actally opened it and closed it.

I'm having real trouble getting this to happen.


What about processes that *don't* do a close - that just use an fd and exit. In the threaded demo I'm looking at, there is only one close -- in the same thread that did the open. The other threads just die.

But sadly for me, the thread that does the open isn't the one that sees the flush() or release() events:


Feb 23 00:17:44 test kernel: DRM(open) pid 1061 open_count = 1
Feb 23 00:17:48 test kernel: radeon_flush: pid = 1064, device = 0xe200, open_count = 2
Feb 23 00:17:48 test kernel: radeon_release: pid = 1064, device = 0xe200, open_count = 2
Feb 23 00:17:48 test kernel: radeon_release: curently hw_lock -795947008 is_held -2147483648 pid 1063 current->pid 1064


The last line indicates that 1063 held the lock. I never see a flush() for that pid.


In other words:
- flush is "process synchronous", and is done for each close() and in the context of the close().


You can have arbitrarily many "flush()" calls for one open, since the process that did the open may have duplicated its file descriptor
through fork() or dup() or even by passing it through UNIX domain sockets.


- release is a memory management "this file no longer exists" thing, and you can't know anything about the context (ie any dependency on _anything_ like "current" etc is a bug).

You can have only one release() call for every open.

NOTE! In general it's a design mistake to care about things like
"current->pid" etc. What are the semantics for the file descriptor for fork'ed processes, or for threads?

Yes, it looks like this is a big mistake throughout the drm code. The assumption seems to be that pids are equivalent to GL contexts - there are heaps of issues I can see with this...


Usually the right answer is that you should _not_ care, and you should _not_ have a flush() call, and the only thing you should care about is the existence of the "struct file".

So it sounds like the fix in DRM is either
 - get rid of all pid dependencies.
 - use flush if you can't.

with the "get rid of pid dependencies" being the better choice if possible.

We do still need some sort of notification of process/fd/GLcontext death to clean up resources owned by that entity - the flush() (and previously release()) methods seemed to do a reasonable job at least for singlethreaded apps -- what would replace these notifications?


Keith



-------------------------------------------------------
This SF.net email is sponsored by: SlickEdit Inc. Develop an edge.
The most comprehensive and flexible code editor you can use.
Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial.
www.slickedit.com/sourceforge
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to