On 9/30/19 8:14 AM, Leonardo Bras wrote:

On Sun, 2019-09-29 at 15:40 -0700, John Hubbard wrote:

quoted

Hi, Leonardo,

Hello John, thanks for the feedback.

quoted

Can we please do it as shown below, instead (compile-tested only)?

This addresses all of the comments that I was going to make about structure
of this patch, which are:

* The lockless synch is tricky, so it should be encapsulated in function
    calls if possible.

As I told before, there are cases where this function is called from
'real mode' in powerpc, which doesn't disable irqs and may have a
tricky behavior if we do. So, encapsulate the irq disable in this
function can be a bad choice.

You still haven't explained how this works in that case. So far, the
synchronization we've discussed has depended upon interrupt disabling
as part of the solution, in order to hold off page splitting and page
table freeing.

Simply skipping that means that an additional mechanism is required...which
btw might involve a new, ppc-specific routine, so maybe this is going to end
up pretty close to what I pasted in after all...

Of course, if we really need that, we can add a bool parameter to the
function to choose about disabling/enabling irqs.

quoted

* This is really a core mm function, so don't hide it away in arch layers.
    (If you're changing mm/ files, that's a big hint.)

My idea here is to let the arch decide on how this 'register' is going
to work, as archs may have different needs (in powerpc for example, we
can't always disable irqs, since we may be in realmode).

Maybe we can create a generic function instead of a dummy, and let it
be replaced in case the arch needs to do so.

Yes, that might be what we need, if it turns out that ppc can't use this
approach (although let's see about that).


thanks,
-- 
John Hubbard
NVIDIA