Bugs in quota code (long) (new)

Jan Kara (jack@atrey.karlin.mff.cuni.cz)
Wed, 14 Apr 1999 01:04:55 +0200


--8t9RHnE3ZwKMSgU+
Content-Type: text/plain; charset=us-ascii

Hello.

It seems like my mail didn't get into the list so I'm resending an
updated version:

So finally I fixed reported race in quotas and I've probably found
some others. So here is the list of them:

1) dquot_transfer() gathers all quotas of inode. When it gathers a quota
it performs a check and then goes for another quota. But dqget() might
block and so the check might be invalid now (some other operation changed
the quota in the meantime). Also when quota_off is run while we
are sleeping the quota might be discarded without writing to disk (see
latter bugs).
I already reported this bug. Fix - lock got quotas...

2) When DQUOT_TRANSFER() is called. It first checked and moved
the quotas, then performed the notify_change() and when it failed
it tried to put quotas back. But this can fail - either we turned
quotas off or the quotas changed while we were sleeping and so
transfer will refuse to return the quotas.
This was also in my previous mail ;-). Fix - move the notify_change()
into the dquot_transfer(). This has a bit unpleasant consequence -
dquot_transfer() now can't return NO_QUOTA / QUOTA_OK but it has
to return error codes so it is a bit inconsistent. But on the other
size every call to dquot_transfer() is done via DQUOT_TRANSFER() do it
doesn't impact any current code...

3) When dqget() is waiting for dquot the dquot might become invalidated.
It is a bit unpleasant for dqget() to return unhashed and invalidated
entries (also some functions doesn't count with it and to they will
happily dereference NULL).
So the dqget() checks dquot after wait_on_dquot() and if it is
invalidated it will return NODQUOT (as if the dquot was invalidated
before the dqget() was called).

4) Functions like dquot_alloc_block/inode also perform the check
and when all checks are OK it starts to update the quotas. Because
update does lock_dquot() it can block. Now the quotas may change
and also for example dquot_transfer() can be run in the meantime
and so we will update other quotas than we should.
We (as in dquot transfer) get all quotas and lock them so nothing
can happen between check and update. Also we have to remove the locks
from dquot_incr_block/inode of course...

5) When quota_off is run, it will drop all pointers to quotas from
inodes. Then invalidate_dquots() goes through list and invalidates
every quota it sees (of course only of specified typed on right device
;-)). But this way we can loose some data - someone (eq. get_quota())
does dqget() - use count is now 2. Now the function blocks (copy_to_
user has to swap?). The quota_off is run. All pointers from inodes
are dropped but not all dquots are written - dqput writes only
dquots where use_count == 0 and our dquot has use count == 1. Then we
invalidate all quotas (we ignore if the dquot is dirty). Then the
function awakes and does dqput(). Now the use_count becomes 0 but
quota is invalidated and so it is not written...
Fix - when invalidate_dquots() sees dirty dquot it writes it first
and then it invalidates it...

6) get_quota() doesn't lock the quota and so it might return some
bad data when copy_to_user blocks (eq. quota_off is run).
Fix - lock it.

So these were the bugs (OK, probably some of these aren't bugs and
I'm missing something...) I found and made a patch for. The patch
is attached to this mail. It is against 2.2.1. For me the patched
quotas work but I'd be glad if someone could try it also in the other
environment..

Here are also some other bugs / suggestions:

7) We already use slab cache to allocate dquots but we *never* free
them. We are just putting them on the free_list. Maybe we can free
some dquots / provide some function like prune_qcache...

8) In case that there is not enough memory and grow_dquots() will not
be able to allocate any other quotas it will be called again and
again. But we have nonempty free list so in fact we have free
quotas...

9) get_empty_dquot() tests whether got dquot isn't locked or dirty.
But locked dquot can't get into free list (dqput always waits for
it). Dirty dquot can be in the free list - when some quota is
invalidated and someone other is performing some action on it
(after my patches this should never happen because every change
is done with locked dquot but..). But in that case writing isn't
the right solution ;-)...
------------------------------------
The new things:
ad 8) Changed grow_dquots() a bit so it will now return number of
successfully allocated structures. If it succeeds, everything restarts.
Otherwise we search the free list.
ad 9) Removed the write from get_empty_dquot()...

Patch also does:
Fixes the second pass in sync_dquots() and invalidate_dquots() - on second
pass we didn't initialize our pointer so in fact we never did more than one
pass.

Added test to sync_dquots for invalidated dquot.

Added correct initialization of wait_queue to clear_dquot.

Added test to dquot_initialize() for dqget() returning NODQUOT.

dqput() now clears DQ_LOCK (after complaining) and DQ_MOD flags
when putting on free list.

Bye, Honza

PS: gzipped diff against 2.2.1 attached...

--8t9RHnE3ZwKMSgU+
Content-Type: application/x-gunzip
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="quota_fix.diff.gz"

H4sICAokEzcAA3F1b3RhX2ZpeC5kaWZmAO1beXPTSBb/2/kUzVQta0eS4yuXMwkwQ9jJEMgO
gZ2tmqVUstV2tLElI8kJXsh+9n1Ht245DgNVe1FFbMvdr7vf8XtHP1uWJWaev/y44/nj2dKV
O/xpErWvGm+XUryQI9Hri25/2N8bDg5F9/DwcMswjPpZl04sXjmh6O2LXpdm7fOsp0+Ftdff
M/eFwS9Pn26JhufHorntzGbB2Pb8wJUt0RwHfhSLKA6X41jQQ7FtiqUfeVNfumIW+FP46Ll2
3DoCEjeB5wKNSSilPQI615uRKM19wPIw11Jbj0PHjyYyhInFKfqzE8chfh5fOWG6caOWgCv9
OFxVUUgOfQf/t/TXE28m7WgVxXJux6uFFJ+2rBrBflgGsRMsUFC/wlmeLUIhQFC9YX8w3O2u
F29m7ovQ47moEcNBdzjop0Lud80DYdBfFLH8GMvQF8RpF2nYqaQqeU0vBYYLfzkfAY+OaunR
rC+ht2UpiigORVHLpFlJKS8VejG3LFH/DwUvXC+U49gLfCVGIOrFMDnAXRj370FrBb/W7MJY
s4vyqqBCO9tbQmyLi4UMHdxcJKLlYhGEMTBqEuCuo2sSfNQm4XY7XbO7Kwx67Sgbnogm78o6
cVkQ1olnRyP4+MEOFi1QSNGgYWeX9i/vLt4+O3t99rY4pyWOj0WnhUMb68jBBzrDzPuHLNIw
YYdknPduyjpJGFyiwcwUHTDZZRjCt9aJBO61Wki4MQ3iQATLmJaRYQhcOhZ+EHuTlQ2S9qd6
V4pQuh8azER+59a6xa0hniSbeRDxhGaRoGjcCTmLJEtvs4PCnC0LeDOEd6GMl6DSNBEhC00t
gaVJtENq3h4XwASBaLcIRJnRrwKfRnd74JiGg4NhdzeFnl7X3BMG/O0OSDXFdqPxRt54EZq9
B8gwd3xnKuewawFCdAhEQmcsIx4L2/vBm83ET86tjEzx/e0VvnkaxU7Y9mV8YorDncMDHAuW
BuNfeB+lIgCGVTTclglPpjJutoTja7BiV9dut5utNhN5Fon4SgqELvlhKf2xpM+Ijx4g1a0T
iXlwg/YYBnNFxZXjkImYTIQfA2Crx3i8MayFFCZLn4AnUgt6/g0Yj+vE0qZpEQz3g1txG3qx
xMVcb+JJteFINAPYTngLTBS3kimMg+XMRWwYO6EromAuBStElBxqFgX8Bc2G00VzGXtjPU4N
+9nxxUsndIDXf3fG10+dOJSr9rUTwsbb88mkPV76Xnv8D+D8YAdVQjBeNX9siR+DxSr0plcx
yn8gLHzZx9hjHIgboPurJ0M4vvQFTtkB9SMA26MQpLs/UP4JtXtnW0QOgMoKtifH1zSaLBaV
xpbzRbxqPiZ2kBWhw2khqoElL2CN+Lr5XShRSOyLaOSQ+YcGI0Bn8QvSwSdP/uZ/x/a66VxU
h6r5bGFHDdz/z+2XbfHzEnTcDfw/4l9QulV8hQpAxwH7a9BxXDkrHQYtFzHZPj+7fGv/dPrs
ecUQcu6dPcR9cO+7ZrdLDNxiX1zWqmtX3tjouG7AW12BTxEYnbS2iHHaq9E5t+nFFNs++EEA
GTD6SNq4Wzxn/dAjFUH6Urp2KNFOY3Jr6v0QZpdJIrt+kqEUHrjDSIxkDL63LS4SNUd+KwrA
RxkhP6+cG4kcFZEEOxVHVovZml0alunglm6vICITzSbvGDATttASj47F63fn58obqm2lTObz
cAC1jzDW7+0rFW2gxfkxHEEeaU+aTqSQD4gr5uYGGzz4UTp67sctZMCzWSgdd5URmysIzJ/g
uYz1S05mzjQSj8XzX+zzix9fnj5Xh2rY9q3jxXbgsw7wFIo06GC9QzrYAAA6OVl+nXvPlj9c
1enoeX7YHf6BUAf+AniIH+TYAW3gsC4i7UERA2SOWPiBP1uJq2DmyjASwYSlpGfjAAqJChMR
PmNAG/iEUVNbXAYAmIyqoGp6erAMIzm7kah4EGFJDaNoqEgaROGIv6MhY/wVLUMJgM6Td7Q8
KyXx6uK5ePxYlJnxiThC+yiIBZ8XFLhL5vGrFNFMLgh9zi5Y0wlBGuOZdMKidOErku+gx+AA
GJuAA9pntPLH/+6o8I1sebdPKr9/UGnLxX8oXK3zVjcvzi+z87PUvust+1GFRjUT4xaflXq1
WsV12a4HHUQqY9AfmN09PmWtR310j0sVqVt0PyyWiSt0FF7VuNKUEsQmEMRaRwWnCAETxX1o
Zmj7NAzAXM4h5XEgXAMrU/xRblX5TNy0ZRXoc6oiPjGuiMvMUdviHCI3QFPGiFsvvoKAWAS3
PoRBHvmSaRCAv3Slk665Ab6i5BRrXp6+eW2fvnkjvvvLi8thfkEGo8WyHDm0E241Sksdi38m
i6WQWTMMVIHY+0pHivgteEiA0kD5x6yYSPiCGPXnGcTKwonB1bgIrOdv3vE4CHyXUuEM7J3B
wp45URZlGrfOtbSXC6U45GtUXDIAn9nrgw7u9szeLutgIkdKPcCqMf6kWGUaBrcaj/ABwKT6
GnGj/C3pZQXg6DLQGDVC9Hu6qsOfO1Rd0AiCz0600mSefQ/zlOJrjLmey7k9dkCdOF1gFvCT
hSkuz5/9YKMKnJ63lAkry+fcUp/bSN7jhggBAasUeEeGQXNhqQhSlObYcd3QjlsKSCH3jSC/
DibN7LFbrD6YfxPzSWpZM1YSYWHD3nUOMQ20NqaIm5E2PcwKOlWBK7D65Bs43ZiMG3cB7/gM
qKq5c95xcYwEmpfaxPNdewQID8z0XY5WbyUmEdJVsiZtGuwiag8OeqpOmXEE8XwBIoK/1kni
Cx6noXuEoG1ZM28OnMiLlZEPYywkYua2hqmijrMZU95eQWwKDhIzLV/eAHhcOYuF9BVm1MZk
lbhdjiHnq2iM0UfWbYUScBATUFcZ1UHH7EKytNs9MA/TWo/WNaFqIQtwmRinAN0pJqHgYK2H
bI4TqerA0Vof8LDCV4U2FgV8FsVcbzGsIiHMnRWi4whyTUzY0RFA5guiwFBNjZ6jvwgl+608
gPKIHXyhk4dyIR1CgTv4X/R5CboXPETET++D/EyGWI33fKQiyjPIiwJ59lqPdIGtmi7RYKo0
/vgPLlAzRYFMEs/vchHQ2O13tX4oO1ToCHajlUPrRII/AHxz56P6oHQgi7wswxKbjXVUtCfN
0eGEIEcI7Vlb5VpIyHOSwWF3D8KcHpz6sG/2e3xsBQ4MWcsFEIE44zfwiFdg7++J5RBPywUo
ePMxfS8TrFRHKsRu4hOlqeBTsYoyCqpSNbH0IUMBLdaRXZriPA8wu4XZQDpNWLCWlKWiMhDI
UijmlW4mz6DoK5spKNG+vniOJVx8dKfZWEAPhmX3A0Jw1J4FwfVywf6mrCGI1UYlVrsf3OVi
5o1hp80K3wti/ZS1IgzJ1N7E58+iIiksH6Cg2LhFowaIaoRUHXvWscsonN6AR4mnosgEvLBy
fMDOdblSEo9Ey4UM+TpFbEcj0OipjG162oSJKj7aG/QpPto77OncO1nY82eeL7M3KlRH5Oy4
ivU1lymcwuFGSvlhhkXLUKXdxrGamB/AQPj5WCE8Wv3SryJ6d/8RiCvf4ghM+BsfgYq8X1MK
BZeQSOIknduoFJWVPSfeCbBO7R0QEO53e8ldUGa6F3tzjC+a+AqRJefTNSnFGdjJ6eW3Y+PX
1IQSG5U21LNRDahhI0WZxn5vYO6VmDh6CBN/OH/5FVlIt5ZUEqFoAfxV6INXa6ZAVLrJNEsQ
nnjNg709iBGMw86+Lg1VQncaVdINGzrH8nZVhYoCtUUYxBJWdKaOh3e/iXvT9TT2fBQx6zss
6/TFs3fnb1NZjmbXGLY/GgeLlR0HNsSFIT82RSa7gSfltAgGqbqIJt9BsK9mc8GrisqrMmTX
4V4HM5Bup9NPC0fwbxRK55orpVyPS9MLvmtKbvzIaXiuiTmRyvprOE5fFcqwNDSlhXN+A0Lv
qfKlJqqabzFUEAVydJxDTs+7nUHP7LOaY6LMFsp3Yl/UEVChgpzroPKixipDTuay9lKemOTq
R6k3jeOVrd5uw3tga3ItCh9r6o6/vXr2V7rWvnzPMQ5Wbpu6CCA4y0/G0APDUCFvHZ8LAsp+
I7KRUcXcVlIPrKGXTwbVJig1sUfuhyqSpEVmwvMMt4ErJjM6W3xovr7gi37eS9EUCttMl37o
iklkjwUDUCNVtPsmIngIQ8sxSCVTky4Uo2pK3cgytGT4mbFF/ZRZYmnREAvsi5c0XiGPfgaP
FCexSE48ZOadaDZalop617AjX2P+gh3nYmWtStpLQZLDnSi9rsKU/UONKYTzRWD5otagcpfO
/wKQCOtEfC0o8b4ASn43iHibg8h/Bnyo+P8h8KGm/B8+1sPHgC4pjW63u6/7hb5OkyKn59VA
UXmJ8A0tne44S0PzVr2JLmaTqHt1sa6CUkFLpWBpOyYq253u1elAFMwywqvFahl9WePnf62M
NsWLTWSUBZJ6Ge33UTQgo92OLi2pLq3XkJoNIQnzIuyzccRI97QFut20zS7bSl32w1tv722v
3TLqyW/SVVsZCSRqkt1g0nuJflR1Xx5l9YxUkFU/EysoTdN7s7HRLx8K1I2Mg+px90UfMFBr
vEkNKDqISVNkVbZMytPco5upbFqnry9OX1MWjV3EWHZ+4eG17jLmqjPIHFvTuTMd71pADZTd
t2n8DmsQQLHZw5687sGuuXuYtEk4kRTvLt/Q6YYVWSmIBc2ChASfHXzQKqWfGE808qxNop9y
wtxM9KglnuRJi6HILq0T6zx9EMjm1LMHGeYXy6XtG+++an/GA/ZXuQVRqDuQXP705s+1cpkW
5TL9dnKZ5uUy/apymRbkMv3dcqmlsKlcprVy4SCtEAoX6auAuFuKhdm/cKQkPn+mi0765YLO
zGsosTmX8/NOjqJyiuxAEW7Yg9K774/J79KHxH3qsC7PYRzyXou2MAC2pb9WPTnBXHL768hx
RVNO21wKDCaTlrpKB0Dmizts08OGULqjgylPMl05VULO3TSVZZj38Z+SRsUKUvlymlF38jTc
rdG6zDWT6t3Jr6j3dv96auQaDU3WErRWbcK2tgfzV8kttZB80dVVUqqNBLcU3GCTPrXWl/sf
K1jwfUkOmvnFHrtsIlnL5aoxOc7QcaiGn4j43jmbrIw3gllGYe8ChhQeduZTtzskrLlLXFZh
FVq1aUIkgfXqMjf5DQI37fNFIf4eIQq4ccALk8h5CTaQLKKmZ5cC851R70FZJI+KR9aXpHQZ
Wz5t5muilIDN74It46vDFumxvhouVwuoCH+XREnNTX4mw4WHHCWRD6Lg0UpAjAx4OSch4TUM
8F+fhn8KkmMqWk3msCbTQjmHcibRZWPz+iKAIA+bmrFJGfUwot8YKPIwYh6EUv3Sq9vfM3sH
EJb1BodmX4dl90Pi48frIbHYQWptCI5Woyo5KU8DPVE+opxuVo3mr3hK2vW40SKVKe3aNcrH
rUbmdPvZkk5Zm/NHzVaPy2NrDnr/ApXV6TX0RaloU4d2a3ydeMgdVCE9ktSjV5lU028MqhLz
ktKywVcfI4V1kXpQWDWRxrpjJT+ru2foA5ev/FLD09ryXaeibleM0opBmm5SpLCJuxRH2Ezn
S/QSoyC+SsEmkrEYLeN8t3MpcquITWpPnwn11geCd9Uhw8MWzAWfFUsWv7/LVyCzv4BURZN+
j34k3cVG08z17s0kmmOODn94D21KoH/Dy/b31Ks2W6g+3tLFLyUKHZM7hFSMXEGQdN+WHxeQ
8SR0Vf0Hcp/0spvbN4ZY4LLPfrHfnr06radKpr8J1VGG6nNN1Xj4XnOyu3fbdQus2XbtApUn
qLwHrxBq8jNcWEc1sSe1sOho619sOfOCkEEAAA==

--8t9RHnE3ZwKMSgU+--

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/