On Sat 09-01-16 18:00:36, Benjamin LaHaise wrote: > On Sat, Jan 09, 2016 at 02:43:44PM -0800, Linus Torvalds wrote: > > On Sat, Jan 9, 2016 at 2:08 PM, Benjamin LaHaise <bcrl@xxxxxxxxx> wrote: > > > > > > Please consider pulling the following changes to fix a couple of issues > > > reported by Dmitry from git://git.kvack.org/~bcrl/aio-fixes.git . > > > > No. This is much too late for this kind of hackery. That second patch > > in particular is both subtle and ugly, and is messing with lockdep. > > I wasn't particularly fond of how Jan implemented that. Any ideas on a > better way to avoid lockdep complaining about this? My initial feedback > to Jan was exactly that this should be handled within the file_start_write() > and file_end_write() APIs -- AIO really shouldn't need to be mucking in what > ought to be hidden behind that API. [To introduce Peter to what we are talking about: This is about filesystem freezing protection implemented effectively as a RW semaphore which must be held for reading whenever we modify filesystem and held for writing when we want to freeze the filesystem. With AIO, we need to return to userspace with the lock held and thus lockdep complains.] Thinking about this some more maybe we could improve lockdep in the following way: We could flag a locking class that it is OK to return with this lock to userspace. When returning to userspace with such lock, lockdep would just silently discard the information about lock being held. When freeing such lock without lockdep knowing we acquired it (i.e., we were called by userspace / IRQ when holding the lock), lockdep would just silently ignore such request. This way we could avoid the hacks with telling lockdep we dropped the lock when we actually did not and we would also get improved coverage to the current state. Peter, would something like the above be doable? Bonus would be if we verified that all locks held when releasing lock we "inherited" rank below the lock we are releasing (that way we would simulate acquiring the lock when entering kernel from userspace). However we would have to be careful about handling locks acquired via trylock (those cannot create deadlocks) or when release happens from softirq / irq - there we care only about locks acquired since softirq started - this happens e.g. when we release the lock on IO completion like in the AIO case. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
