On Fri, 29 Jan 2016, Davidlohr Bueso wrote: > On Wed, 27 Jan 2016, Hugh Dickins wrote: > > > > + * > > > + * The RCU read lock is taken as the inode is finally freed > > > + * under RCU. If the mapping still matches expectations then > > > the > > > + * mapping->host can be safely accessed as being a valid > > > inode. > > > + */ > > > + rcu_read_lock(); > > > + if (READ_ONCE(page->mapping) != mapping || > > > + !mapping->host) { > > > > If you're being as paranoid as all the WARN_ON_ONCEs hereabouts imply, > > then it would be better to do the inode = READ_ONCE(mapping->host) > > before checking !inode rather than !mapping->host. > > Ok, it also reads a bit nicer than the above, which was simply avoiding > a load in the again case. > > rcu_read_lock(); > inode = READ_ONCE(mapping->host);
Just a quick unthinking from-the-hip response to that, something for you to think over before sending v5: without looking back at the code, it's not obvious to me that it's safe to read mapping->host before we've confirmed under rcu_read_lock() that page->mapping still matches mapping. > > if (!inode || READ_ONCE(page->mapping) != mapping) > rcu_read_unlock(); > put_page(page); > > goto again; > }