[RFC] inode.i_opflags - Usage of two different locking schemes

[RFC] inode.i_opflags - Usage of two different locking schemes

[Alexander Lochmann]

[-- Attachment #1.1: Type: text/plain, Size: 939 bytes --]

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.

Regards,
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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC] inode.i_opflags - Usage of two different locking schemes

[Theodore Ts'o]

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

Re: [RFC] inode.i_opflags - Usage of two different locking schemes

[Alexander Lochmann]

[-- Attachment #1.1: Type: text/plain, Size: 643 bytes --]



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 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC] inode.i_opflags - Usage of two different locking schemes

[Theodore Ts'o]

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

[Alexander Lochmann]

[-- Attachment #1.1: Type: text/plain, Size: 1512 bytes --]



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 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC] inode.i_opflags - Usage of two different locking schemes

[Jan Kara]

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 <jack@suse.com>
SUSE Labs, CR

Re: [RFC] inode.i_opflags - Usage of two different locking schemes

[Alexander Lochmann]

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