On 12/03/2020 15:11, Jason Gunthorpe wrote:
On Thu, Mar 12, 2020 at 02:40:08PM +0000, Steven Price wrote:
On 12/03/2020 14:27, Jason Gunthorpe wrote:
On Thu, Mar 12, 2020 at 10:28:13AM +0000, Steven Price wrote:
By refactoring to deal with the !pud_huge(pud) || !pud_devmap(pud)
condition early it's possible to remove the 'ret' variable and remove a
level of indentation from half the function making the code easier to
read.
No functional change.
Signed-off-by: Steven Price <steven.price@xxxxxxx>
Thanks to Jason's changes there were only two code paths left using
the out_unlock label so it seemed like a good opportunity to
refactor.
Yes, I made something very similar, what do you think of this:
https://github.com/jgunthorpe/linux/commit/93f0ed42ab3f9ceb27b58fb7c7c3ecaf60f16b36
Even better! Sorry I didn't realise you'd already done this. I just saw that
the function was needlessly complicated after your fix, so I thought I'd do
a drive-by cleanup since part of the mess was my fault! :)
No worries, I've got a lot of patches for hmm_range_fault right now,
just trying to organize them, test them and post them. Haven't posted
that one yet.
Actually, while you are looking at this, do you think we should be
adding at least READ_ONCE in the pagewalk.c walk_* functions? The
multiple references of pmd, pud, etc without locking seems sketchy to
me.
I agree it seems worrying. I'm not entirely sure whether the holding of
mmap_sem is sufficient, this isn't something that I changed so I've just
been hoping that it's sufficient since it seems to have been working
(whether that's by chance because the compiler didn't generate multiple
reads I've no idea). For walking the kernel's page tables the lack of
READ_ONCE is also not great, but at least for PTDUMP we don't care too
much about accuracy and it should be crash proof because there's no RCU
grace period. And again the code I was replacing didn't have any special
protection.
I can't see any harm in updating the code to include READ_ONCE and I'm
happy to review a patch.
Thanks,
Steve
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
