Date: Mon, 19 Apr 2021 22:06:40 +0200 From: Alexander Lochmann <alexander.lochmann@tu-dortmund.de> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-fs@freebsd.org Subject: Re: [struct buf] Unlocked access to b_vflags? Message-ID: <6f370c67-f3ba-00ce-5da7-7ee400a4180b@tu-dortmund.de> In-Reply-To: <YH3hQcmVhFvQK81W@kib.kiev.ua> References: <792c8a3d-8ea6-073f-3fda-b3eb793ef2b9@tu-dortmund.de> <YHVxfMrU9lmw3sG9@kib.kiev.ua> <4ade0f5d-d4f4-616a-b198-fc58f947070d@tu-dortmund.de> <YH3hQcmVhFvQK81W@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--HFWgVQWvOw8vw2RbNT5bdXY3emhVZWJKN
Content-Type: multipart/mixed; boundary="50Rj90fW8Fc0EkjBE07aP6mNyo9ZDIM1h";
protected-headers="v1"
From: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
To: Konstantin Belousov <kostikbel@gmail.com>
Cc: freebsd-fs@freebsd.org
Message-ID: <6f370c67-f3ba-00ce-5da7-7ee400a4180b@tu-dortmund.de>
Subject: Re: [struct buf] Unlocked access to b_vflags?
References: <792c8a3d-8ea6-073f-3fda-b3eb793ef2b9@tu-dortmund.de>
<YHVxfMrU9lmw3sG9@kib.kiev.ua>
<4ade0f5d-d4f4-616a-b198-fc58f947070d@tu-dortmund.de>
<YH3hQcmVhFvQK81W@kib.kiev.ua>
In-Reply-To: <YH3hQcmVhFvQK81W@kib.kiev.ua>
--50Rj90fW8Fc0EkjBE07aP6mNyo9ZDIM1h
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: de-DE
Content-Transfer-Encoding: quoted-printable
On 19.04.21 22:00, Konstantin Belousov wrote:
> On Mon, Apr 19, 2021 at 09:43:47PM +0200, Alexander Lochmann wrote:
>> Out of curiosity: What's the path from function entry to the access in=20
line
>> 759?
>> The lock is acquired upon function entry in line 7197, and never relea=
sed
>> afterwards (except for the lines 7212 and 7217).
> The bufobj mutex is interlocked with the buffer locks. Basically, each=
> time LK_INTERLOCK is used, the bo_lock is dropped.
Is this done by BUF_LOCK() in line 7233?
Is the interlocked behavior done automatically, or just because both=20
LK_INTERLOCK and BO_LOCKPTR(bo) are specified?
>
> Or do you ask about something else?
No. I think that's it.
- Alex
>=20
>>
>> - Alex
>>
>> On 13.04.21 12:25, Konstantin Belousov wrote:
>>> On Mon, Apr 12, 2021 at 11:19:05PM +0200, Alexander Lochmann wrote:
>>>> Hi folks,
>>>>
>>>> I'm was digging through our data set when I encountered a strange si=
tuation:
>>>> According to the code in trunc_dependencies() in sys/ufs/ffs/ffs_sof=
tdep.c,
>>>> the bo_lock should be held. At least that's how I read the code.
>>>> However, we see several thousands of accesses to b_vflags without th=
e
>>>> bo_lock held.
>>>> At least the own b_lock is acquired.
>>>> The access happens in line 7549: bp->b_vflags |=3D BV_SCANNED; [1]
>>>> Can you please shed some light on this situation?
>>>> Is the b_lock sufficeint, and somehow overrules the bo_lock?
>>>> Am I missing something?
>>> I think you found a valid race. There is one more place where BV_SCA=
NNED
>>> was manipulated without owning bufobj lock. Patch below should fix b=
oth.
>>>
>>> commit a678470b1307542c5a46b930c119b2358863e0d2
>>> Author: Konstantin Belousov <kib@FreeBSD.org>
>>> Date: Tue Apr 13 13:22:56 2021 +0300
>>>
>>> b_vflags update requries bufobj lock
>>> Reported by: Alexander Lochmann <alexander.lochmann@tu-dortm=
und.de> (trunc_dependencies())
>>>
>>> diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
>>> index 0091b5dcd3b8..23c0cf6e128b 100644
>>> --- a/sys/ufs/ffs/ffs_softdep.c
>>> +++ b/sys/ufs/ffs/ffs_softdep.c
>>> @@ -7546,7 +7546,9 @@ trunc_dependencies(ip, freeblks, lastlbn, lasto=
ff, flags)
>>> BO_LOCK(bo);
>>> goto cleanrestart;
>>> }
>>> + BO_LOCK(bo);
>>> bp->b_vflags |=3D BV_SCANNED;
>>> + BO_UNLOCK(bo);
>>> bremfree(bp);
>>> if (blkoff !=3D 0) {
>>> allocbuf(bp, blkoff);
>>> diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c
>>> index dc638595eb7b..05eb19c0ee13 100644
>>> --- a/sys/ufs/ffs/ffs_vnops.c
>>> +++ b/sys/ufs/ffs/ffs_vnops.c
>>> @@ -321,8 +321,9 @@ ffs_syncvnode(struct vnode *vp, int waitfor, int =
flags)
>>> if (BUF_LOCK(bp,
>>> LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK,
>>> BO_LOCKPTR(bo)) !=3D 0) {
>>> + BO_LOCK(bo);
>>> bp->b_vflags &=3D ~BV_SCANNED;
>>> - goto next;
>>> + goto next_locked;
>>> }
>>> } else
>>> continue;
>>> @@ -385,6 +386,7 @@ ffs_syncvnode(struct vnode *vp, int waitfor, int =
flags)
>>> * to start from a known point.
>>> */
>>> BO_LOCK(bo);
>>> +next_locked:
>>> nbp =3D TAILQ_FIRST(&bo->bo_dirty.bv_hd);
>>> }
>>> if (waitfor !=3D MNT_WAIT) {
>>>
>>
>> --=20
>> Technische Universit=C3=A4t 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
>>
>=20
>=20
>=20
--=20
Technische Universit=C3=A4t 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
--50Rj90fW8Fc0EkjBE07aP6mNyo9ZDIM1h--
--HFWgVQWvOw8vw2RbNT5bdXY3emhVZWJKN
Content-Type: application/pgp-signature; name="OpenPGP_signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="OpenPGP_signature"
-----BEGIN PGP SIGNATURE-----
wsF5BAABCAAjFiEElhZsUHzVP0dbkjCRWT7tBbw+9v0FAmB94tAFAwAAAAAACgkQWT7tBbw+9v0N
JQ//aeEs/I94nNXCWGFUfEMGlF18dTGS2wAhRozhAs3SargGtMkjPWOD25NwCgZdhwGksnGdcmHY
nCTTN3NVlHp801+puXx7Q4UEfWANSn1L1LlEy/XJl+5K811xxS8+qfQoBz+coMdmO1OwbRmQUppB
5hIJjRdHrG+9SIfYlXC31xkVDOMPUpXJpCpC4Nm2mdc7oW0AGDFhiUvZwAE1ZnvdGxYIopIdiTHa
kY9lmSIqukw4KNkoF2TmTuBuJNCrIM6i/aC3nJliRapwUPAvwUm54Su8pOkQoOPLBeQXgsvv/xx6
XQw1KXh7d9K1J7/BQ2+u3pTXEd9tr07HvGkR7LG+EYLZ1K9E027wBf7jJHQmgEXzaNMZNII1Bvq2
pFAgyHBQin837jAlFbSv3DLnpQKtLw7Ylg07UjQXZiKgBRFXqgiCpcMG37rFNjqzPVAYDrtLM51k
bXw9Fgf3Md76qpg0tr14f2O0ICN5yQ0uphPktuQAF4ZXZsdDSH3+Au8fR9eD9sIkhGAf+Ay0TeY7
fYB0Ossd4cDBWQX3wWReSdkGNeJxLETNnxN7TBLLpcpi8IYVgXHIbyC41Lb5wFXXgaUcu5sWxCOJ
NSLqud5WsKHoO9z26wTqhXCGnaq5CJydSgjXfA1QDGh8E/M1li7CYdsmLrKeh8RkTleFrBuK4wec
Qs0=
=CfoM
-----END PGP SIGNATURE-----
--HFWgVQWvOw8vw2RbNT5bdXY3emhVZWJKN--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6f370c67-f3ba-00ce-5da7-7ee400a4180b>