Al Viro <v...@zeniv.linux.org.uk> writes: > On Tue, Dec 19, 2017 at 07:40:43PM +0100, Giuseppe Scrivano wrote: >> Giuseppe Scrivano <gscri...@redhat.com> writes: >> >> > The only issue I've seen with my version is that if I do: >> > >> > # unshare -im /bin/sh >> > # mount -t mqueue mqueue /dev/mqueue >> > # touch /dev/mqueue/foo >> > # umount /dev/mqueue >> > # mount -t mqueue mqueue /dev/mqueue >> > >> > then /dev/mqueue/foo doesn't exist at this point. Your patch does not >> > have this problem and /dev/mqueue/foo is again accessible after the >> > second mount. >> >> although, how much is that of an issue? Is there any other way to delay > > You do realize that with your patch you would end up with worse than that - > mount/touch/umount and now you've got ->mq_sb non-NULL and pointing to freed > super_block. With really unpleasant effects when you quit that ipcns... > >> the cost of kern_mount_data()? Most containers have /dev/mqueue mounted >> but it is not really going to be used. > > _What_ cost? At mount(2) time you are setting the superblock up anyway, so > what would you be delaying? kmem_cache_alloc() for struct mount and > assignments > to its fields? That's noise; if anything, I would expect the main cost with > a plenty of containers to be in sget() scanning the list of mqueue > superblocks. > And we can get rid of that, while we are at it - to hell with mount_ns(), with > that approach we can just use mount_nodev() instead. The logics in > mq_internal_mount() will deal with multiple instances - if somebody has > already > triggered creation of internal mount, all subsequent calls in that ipcns will > end up avoiding kern_mount_data() entirely. And if you have two callers > racing - sure, you will get two superblocks. Not for long, though - the first > one to get to setting ->mq_mnt (serialized on mq_lock) wins, the second loses > and prompty destroys his vfsmount and superblock. I seriously suspect that > variant below would cut down on the cost a whole lot more - as it is, we have > the total of O(N^2) spent in the loop inside of sget_userns() when we create > N ipcns and mount in each of those; this patch should cut that to O(N)...
Thanks for the explanation. I see what you mean now and how this cost is inevitable as anyway we need to setup the superblock. Giuseppe
