On Fri, Jan 8, 2021 at 4:48 AM Will Deacon <will@xxxxxxxxxx> wrote: > > It certainly looks simple and correct to me, although it means we're now > taking the mmap sem for write in the case where we only want to clear the > access flag, which should be fine with the thing only held for read, no? When I was looking at that code, I was thinking that the whole function should be split up to get rid of some of the indentation and the "goto out_mm". And yes, it would probably be good to split up up even more than that "initial mm lookup and error handling", and have an actual case statement for the different clear_ref 'type' cases. And then it would be fairly simple and clean to say "this case only needs the mmap_sem for read, that case needs it for write". So I don't disagree, but I think it should be a separate patch - if it even matters. Is this strange /proc case something that is even commonly done? Linus