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 <[email protected]>
SUSE Labs, CR
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 Lochmann PGP 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