Re: steved patches applied to 2.5.68-bk4

[John L. <le...@mo...>]

On Mon, Apr 28, 2003 at 09:12:58AM -0400, Steve Dickson wrote:

> 1) Added  O_NDELAY process. It seemed like a good idea.

Good ideas generally have reasons ... and as your "XXX" comments, it
doesn't even work properly. So what is the point ?

> 2) You can not hold the buffer_sem lock during the copy_to_user
>      because it could cause a deadlock in the do_ummap() code
>      during a page fault

As I mentioned I don't want to vmalloc() very large regions of memory on
every read() just to avoid a theoretical race. At the very least a
solution like this must be accompanied by benchmarks proving it's not a
problem.

Other possibilities to fix the theoretical bug are to schedule the
buffer sync for later processing instead of calling sync_buffers()
directly, or calling the callback outside of the mmap_sem. The first
will lose samples, the second will cause extra overhead as we can't see
if the unmap was of an executable section.

Other ideas are welcome, all of them have drawbacks. But right now I
would be tempted to go the moving out of the mmap_sem route, and add
a check if the region is executable inside the oprofile code itself. A
simple check (find_vma_prev on the address and then see if that is
executable) will probably be good enough.

> 3) You have to check buffer_ready while holding the lock
>      to ensure you catch two racing threads.

It's impossible to work out which bit you mean since you combined three
changes in one patch. But anyway, I don't see this race, please give a
diagram showing the claimed race.

john