In terms of an alternative, I feel that we should:
1. Continue to not change external APIs (correct to not do so)
2. Avoid adding this to any code paths that don't strictly need it
3. Refactor the existing code to make it harder to get wrong. Don't have an
easily confused sentinel, rather have some kind of context state that is
passed through.
I mean we already have alloc_context and you're already updating it :)
But instead of overloading user_addr to indicate all kinds of things, instead
make life easier by actually breaking things out.
Like:
enum alloc_context_type {
KERNEL_ALLOCATION,
USER_MAPPED_ALLOCATION,
USER_UNMAPPED_ALLOCATION, // Maybe? Do we ever?
/* Perhaps some other states we want to encode? */
};
struct alloc_context {
...
enum alloc_context_type type;
unsigned long user_addr; // Only set if type == USER_ALLOCATION
// Maybe something suggesting context or whether we init before in some
// cases?
};
And thread that through?
That way we can also add further context later if required rather than this
awkward easily misunderstood parameter.
It might means some further prepatory patches but it avoids the confusion of not
knowing what to set this to, and could perhaps be set further up the stack?
Then look to completely eliminate cases where we zero other than with
__GFP_ZERO.
Perhaps others have ideas too, but this kind of approach seems more appropriate?
It also seems right to send this work as a entirely separate series. 'Always
zero user memory using __GFP_ZERO' seems like a totally valid independent
series.
Thanks, Lorenzo
