Messages in this thread | | | Date | Wed, 20 May 2020 19:26:28 -0700 | From | Matthew Wilcox <> | Subject | Re: [PATCH v3] /dev/mem: Revoke mappings when a driver claims the region |
| |
On Wed, May 20, 2020 at 06:35:25PM -0700, Dan Williams wrote: > +static struct inode *devmem_inode; > + > +#ifdef CONFIG_IO_STRICT_DEVMEM > +void revoke_devmem(struct resource *res) > +{ > + struct inode *inode = READ_ONCE(devmem_inode); > + > + /* > + * Check that the initialization has completed. Losing the race > + * is ok because it means drivers are claiming resources before > + * the fs_initcall level of init and prevent /dev/mem from > + * establishing mappings. > + */ > + smp_rmb(); > + if (!inode) > + return;
But we don't need the smp_rmb() here, right? READ_ONCE and WRITE_ONCE are a DATA DEPENDENCY barrier (in Documentation/memory-barriers.txt parlance) so the smp_rmb() is superfluous ...
> + /* > + * Use a unified address space to have a single point to manage > + * revocations when drivers want to take over a /dev/mem mapped > + * range. > + */ > + inode->i_mapping = devmem_inode->i_mapping; > + inode->i_mapping->host = devmem_inode;
umm ... devmem_inode->i_mapping->host doesn't already point to devmem_inode?
> + > + /* publish /dev/mem initialized */ > + smp_wmb(); > + WRITE_ONCE(devmem_inode, inode);
As above, unnecessary barrier, I think.
| |