On Mar 6, 2023, at 5:19 PM, Peter Xu peterx@redhat.com wrote:
!! External Email
On Mon, Mar 06, 2023 at 02:50:23PM -0800, Axel Rasmussen wrote:
We have a lot of functions which take an address + length pair, currently passed as separate arguments. However, in our userspace API we already have struct uffdio_range, which is exactly this pair, and this is what we get from userspace when ioctls are called.
Instead of splitting the struct up into two separate arguments, just plumb the struct through to the functions which use it (once we get to the mfill_atomic_pte level, we're dealing with single (huge)pages, so we don't need both parts).
Relatedly, for waking, just re-use this existing structure instead of defining a new "struct uffdio_wake_range".
Signed-off-by: Axel Rasmussen axelrasmussen@google.com
fs/userfaultfd.c | 107 +++++++++++++--------------------- include/linux/userfaultfd_k.h | 17 +++--- mm/userfaultfd.c | 92 ++++++++++++++--------------- 3 files changed, 96 insertions(+), 120 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index b8e328123b71..984b63b0fc75 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -95,11 +95,6 @@ struct userfaultfd_wait_queue { bool waken; };
-struct userfaultfd_wake_range {
unsigned long start;unsigned long len;-};
Would there still be a difference on e.g. 32 bits systems?
[...]
static __always_inline int validate_range(struct mm_struct *mm,
__u64 start, __u64 len)
const struct uffdio_range *range){ __u64 task_size = mm->task_size;
if (start & ~PAGE_MASK)
if (range->start & ~PAGE_MASK) return -EINVAL;
if (len & ~PAGE_MASK)
if (range->len & ~PAGE_MASK) return -EINVAL;
if (!len)
if (!range->len) return -EINVAL;
if (start < mmap_min_addr)
if (range->start < mmap_min_addr) return -EINVAL;
if (start >= task_size)
if (range->start >= task_size) return -EINVAL;
if (len > task_size - start)
if (range->len > task_size - range->start) return -EINVAL;}
Personally I don't like a lot on such a change. :( It avoids one parameter being passed over but it can add a lot indirections.
Do you strongly suggest this? Shall we move on without this so to not block the last patch (which I assume is the one you're looking for)?
Just in case you missed, it is __always_inline, so I presume that from a generated code point-of-view it is the same.
Having said that, small assignments to local start, let and range variables would make the code easier to read and reduce the change-set.