Hi Pavel, On Mon, 18 May 2020, Pavel Machek wrote: > Hi! > > > This may not risk an actual deadlock, since shmem inodes do not take > > part in writeback accounting, but there are several easy ways to avoid > > it. > > ... > > > Take info->lock out of the chain and the possibility of deadlock or > > lockdep warning goes away. > > It is unclear to me if actual possibility of deadlock exists or not, > but anyway: > > > int retval = -ENOMEM; > > > > - spin_lock_irq(&info->lock); > > + /* > > + * What serializes the accesses to info->flags? > > + * ipc_lock_object() when called from shmctl_do_lock(), > > + * no serialization needed when called from shm_destroy(). > > + */ > > if (lock && !(info->flags & VM_LOCKED)) { > > if (!user_shm_lock(inode->i_size, user)) > > goto out_nomem; > > Should we have READ_ONCE() here? If it is okay, are concurency > sanitizers smart enough to realize that it is okay? Replacing warning > with different one would not be exactly a win... If a sanitizer comes to question this change, I don't see how a READ_ONCE() anywhere near here (on info->flags?) is likely to be enough to satisfy it - it would be asking for a locking scheme that it understands (being unable to read the comment) - and might then ask for that same locking in the numerous other places that read info->flags (and a few that write it). Add data_race()s all over? (Or are you concerned about that inode->i_size, which I suppose ought really to be i_size_read(inode) on some 32-bit configurations; though that's of very long standing, and has never caused any concern before.) I am not at all willing to add annotations speculatively, in case this or that tool turns out to want help later. So far I've not heard of any such complaint on 5.7-rc[3456] or linux-next: but maybe this is too soon to hear a complaint, and you feel this should not be rushed into -stable? This was an AUTOSEL selection, to which I have no objection, but it isn't something we were desperate to push into -stable: so I've also no objection if Greg shares your concern, and prefers to withdraw it. (That choice may depend on to what extent he expects to be keeping -stable clean against upcoming sanitizers in future.) Hugh