Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On 16.03.21 18:14, Jan Kara wrote: > > So i_lock is supposed to protect i_opflags for writing AFAICT. For reading > we don't seem to bother in some cases and I agree that is potentially > problematic. It is *mostly* OK because we initialize i_opflags when loading > inode into memory / adding it to dcache. But sometimes we also update them > while the inode is alive. Now this is fine for the particular flag we > update but in theory, if the compiler wants to screw us and stores > temporarily some nonsensical value in i_opflags we'd have a problem. This > is mostly a theoretical issue but eventually we probably want to fix this. > > Honza > Thx for the detailed explanation. :-) - Alex -- Technische Universität Dortmund Alexander LochmannPGP key: 0xBC3EF6FD Otto-Hahn-Str. 16 phone: +49.231.7556141 D-44227 Dortmund fax:+49.231.7556116 http://ess.cs.tu-dortmund.de/Staff/al
Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On Mon 08-03-21 15:05:33, Alexander Lochmann wrote: > On 05.03.21 17:04, Theodore Ts'o wrote: > > On Fri, Mar 05, 2021 at 04:35:47PM +0100, Alexander Lochmann wrote: > > > > > > > > > On 05.03.21 16:18, Theodore Ts'o wrote: > > > > 1) I don't see where i_opflags is being read in ipc/mqueue.c at all, > > > > either with or without i_rwsem. > > > > > > > It is read in fs/dcache.c > > > > So why is this unique to the mqueue inode then? It might be helpful > > to have explicit call stacks in the e-mail, in text form, when you > > resend to LKML. > It is unique to mqeue inode, because the control flow goes through > ipc/mqueue.c where almost always the i_rwsem is taken. > Hence, we see more memory accesses to an mqueue inode with the i_rwsem. > The i_lock is less often hold compared to the i_rwsem. > We conclude the i_rwsem is needed. So it might not be a contradiction at > all. It rather could be a flaw in our approach. :-/ > > Besides from our current discussion: > Does the i_lock protect i_opflags for both reading and writing? So i_lock is supposed to protect i_opflags for writing AFAICT. For reading we don't seem to bother in some cases and I agree that is potentially problematic. It is *mostly* OK because we initialize i_opflags when loading inode into memory / adding it to dcache. But sometimes we also update them while the inode is alive. Now this is fine for the particular flag we update but in theory, if the compiler wants to screw us and stores temporarily some nonsensical value in i_opflags we'd have a problem. This is mostly a theoretical issue but eventually we probably want to fix this. Honza -- Jan Kara SUSE Labs, CR
Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On 05.03.21 17:04, Theodore Ts'o wrote: On Fri, Mar 05, 2021 at 04:35:47PM +0100, Alexander Lochmann wrote: On 05.03.21 16:18, Theodore Ts'o wrote: 1) I don't see where i_opflags is being read in ipc/mqueue.c at all, either with or without i_rwsem. It is read in fs/dcache.c So why is this unique to the mqueue inode then? It might be helpful to have explicit call stacks in the e-mail, in text form, when you resend to LKML. It is unique to mqeue inode, because the control flow goes through ipc/mqueue.c where almost always the i_rwsem is taken. Hence, we see more memory accesses to an mqueue inode with the i_rwsem. The i_lock is less often hold compared to the i_rwsem. We conclude the i_rwsem is needed. So it might not be a contradiction at all. It rather could be a flaw in our approach. :-/ Besides from our current discussion: Does the i_lock protect i_opflags for both reading and writing? Cheers, Alex That's because the HTML file is ***huge*** (1.7Meg), and I'm having trouble with my browser properly rendering it. In any case, the html claims to be showing the counter examples and I'm still stuck on the *example*? Cheers, - Ted -- Technische Universität Dortmund Alexander LochmannPGP key: 0xBC3EF6FD Otto-Hahn-Str. 16 phone: +49.231.7556141 D-44227 Dortmund fax:+49.231.7556116 http://ess.cs.tu-dortmund.de/Staff/al OpenPGP_signature Description: OpenPGP digital signature
Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On Fri, Mar 05, 2021 at 04:35:47PM +0100, Alexander Lochmann wrote: > > > On 05.03.21 16:18, Theodore Ts'o wrote: > > 1) I don't see where i_opflags is being read in ipc/mqueue.c at all, > > either with or without i_rwsem. > > > It is read in fs/dcache.c So why is this unique to the mqueue inode then? It might be helpful to have explicit call stacks in the e-mail, in text form, when you resend to LKML. That's because the HTML file is ***huge*** (1.7Meg), and I'm having trouble with my browser properly rendering it. In any case, the html claims to be showing the counter examples and I'm still stuck on the *example*? Cheers, - Ted
Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On 05.03.21 16:18, Theodore Ts'o wrote: 1) I don't see where i_opflags is being read in ipc/mqueue.c at all, either with or without i_rwsem. It is read in fs/dcache.c 2) I'm not sure what this has to do with ext4? - Ted Yeah. You're right. That was my fault. Sry. I should have sent it to linux-kernel@... only. - Alex -- Technische Universität Dortmund Alexander LochmannPGP key: 0xBC3EF6FD Otto-Hahn-Str. 16 phone: +49.231.7556141 D-44227 Dortmund fax:+49.231.7556116 http://ess.cs.tu-dortmund.de/Staff/al OpenPGP_signature Description: OpenPGP digital signature
Re: [RFC] inode.i_opflags - Usage of two different locking schemes
On Fri, Mar 05, 2021 at 02:10:09PM +0100, Alexander Lochmann wrote: > Hi folks, > > I've stumbled across an interesting locking scheme. It's related to struct > inode, more precisely it is an mqueue inode. > Our results show that inode:mqueue.i_opflags is read with i_rwsem being > hold. > In d_flags_for_inode, and do_inode_permission the i_lock is used to read and > write i_opflags. > Is this a real locking scheme? Is a lock needed to access i_opflags at all? > What is the magic behind this contradiction? > > I've put the report of the counterexamples on our webserver: > https://ess.cs.tu-dortmund.de/lockdoc-bugs/cex-inode-mqueue.html. > It contains the stacktraces leading to those accesses, and the locks that > were actually held. 1) I don't see where i_opflags is being read in ipc/mqueue.c at all, either with or without i_rwsem. 2) I'm not sure what this has to do with ext4? - Ted
