On Thu, Oct 15, 2020 at 9:52 AM Daniel Vetter daniel.vetter@ffwll.ch wrote:
On Thu, Oct 15, 2020 at 2:09 AM Jason Gunthorpe jgg@ziepe.ca wrote:
On Fri, Oct 09, 2020 at 11:28:54AM -0700, Dan Williams wrote:
On Fri, Oct 9, 2020 at 7:32 AM Jason Gunthorpe jgg@ziepe.ca wrote:
On Fri, Oct 09, 2020 at 04:24:45PM +0200, Daniel Vetter wrote:
On Fri, Oct 9, 2020 at 2:31 PM Jason Gunthorpe jgg@ziepe.ca wrote:
On Fri, Oct 09, 2020 at 09:59:31AM +0200, Daniel Vetter wrote:
> +struct address_space *iomem_get_mapping(void) > +{ > + return iomem_inode->i_mapping;
This should pair an acquire with the release below
> + /* > + * Publish /dev/mem initialized. > + * Pairs with smp_load_acquire() in revoke_iomem(). > + */ > + smp_store_release(&iomem_inode, inode);
However, this seems abnormal, initcalls rarely do this kind of stuff with global data..
The kernel crashes if this fs_initcall is raced with iomem_get_mapping() due to the unconditional dereference, so I think it can be safely switched to a simple assignment.
Ah yes I checked this all, but forgot to correctly annotate the iomem_get_mapping access. For reference, see b34e7e298d7a ("/dev/mem: Add missing memory barriers for devmem_inode").
Oh yikes, so revoke_iomem can run concurrently during early boot, tricky.
It runs early because request_mem_region() can run before fs_initcall. Rather than add an unnecessary lock just arrange for the revoke to be skipped before the inode is initialized. The expectation is that any early resource reservations will block future userspace mapping attempts.
Actually, on this point a simple WRITE_ONCE/READ_ONCE pairing is OK, Paul once explained that the pointer chase on the READ_ONCE side is required to be like an acquire - this is why rcu_dereference is just READ_ONCE
Indeed this changed with the sm_read_barrier_depends() removal a year ago. Before that READ_ONCE and rcu_dereference where not actually the same. I guess I'll throw a patch on top to switch that over too.
Actually 2019 landed just the clean-up, the read change landed in 2017 already:
commit 76ebbe78f7390aee075a7f3768af197ded1bdfbb Author: Will Deacon will@kernel.org Date: Tue Oct 24 11:22:47 2017 +0100
locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()
Cheers, Daniel