Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-10 Thread Suren Baghdasaryan
On Wed, Aug 9, 2023 at 2:07 PM Mateusz Guzik wrote: > > On 8/5/23, Suren Baghdasaryan wrote: > > On Fri, Aug 4, 2023 at 6:06 PM Mateusz Guzik wrote: > >> > >> On 8/5/23, Linus Torvalds wrote: > >> > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: > >> >> > >> >> I know of these guys, I

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-09 Thread Mateusz Guzik
On 8/5/23, Suren Baghdasaryan wrote: > On Fri, Aug 4, 2023 at 6:06 PM Mateusz Guzik wrote: >> >> On 8/5/23, Linus Torvalds wrote: >> > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: >> >> >> >> I know of these guys, I think they are excluded as is -- they go >> >> through access_remote_vm,

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Suren Baghdasaryan
On Fri, Aug 4, 2023 at 6:06 PM Mateusz Guzik wrote: > > On 8/5/23, Linus Torvalds wrote: > > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: > >> > >> I know of these guys, I think they are excluded as is -- they go > >> through access_remote_vm, starting with: > >> if

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Suren Baghdasaryan
On Fri, Aug 4, 2023 at 6:17 PM Mateusz Guzik wrote: > > On 8/5/23, Suren Baghdasaryan wrote: > > On Fri, Aug 4, 2023 at 5:49 PM Mateusz Guzik wrote: > >> However, the other users (that I know of ) go through the mmap > >> semaphore to mess with anything which means they will wait for > >>

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Mateusz Guzik
On 8/5/23, Suren Baghdasaryan wrote: > On Fri, Aug 4, 2023 at 5:49 PM Mateusz Guzik wrote: >> However, the other users (that I know of ) go through the mmap >> semaphore to mess with anything which means they will wait for >> dup_mmap to finish (or do their work first). I would be surprised if

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Suren Baghdasaryan
On Fri, Aug 4, 2023 at 5:49 PM Mateusz Guzik wrote: > > On 8/5/23, Suren Baghdasaryan wrote: > > On Fri, Aug 4, 2023 at 5:26 PM Suren Baghdasaryan > > wrote: > >> > >> On Fri, Aug 4, 2023 at 5:15 PM Linus Torvalds > >> wrote: > >> > > >> > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: >

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Mateusz Guzik
On 8/5/23, Linus Torvalds wrote: > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: >> >> I know of these guys, I think they are excluded as is -- they go >> through access_remote_vm, starting with: >> if (mmap_read_lock_killable(mm)) >> return 0; >> >> while dup_mmap

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Mateusz Guzik
On 8/5/23, Suren Baghdasaryan wrote: > On Fri, Aug 4, 2023 at 5:26 PM Suren Baghdasaryan > wrote: >> >> On Fri, Aug 4, 2023 at 5:15 PM Linus Torvalds >> wrote: >> > >> > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: >> > > >> > > I know of these guys, I think they are excluded as is --

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Suren Baghdasaryan
On Fri, Aug 4, 2023 at 5:26 PM Suren Baghdasaryan wrote: > > On Fri, Aug 4, 2023 at 5:15 PM Linus Torvalds > wrote: > > > > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: > > > > > > I know of these guys, I think they are excluded as is -- they go > > > through access_remote_vm, starting

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Suren Baghdasaryan
On Fri, Aug 4, 2023 at 5:15 PM Linus Torvalds wrote: > > On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: > > > > I know of these guys, I think they are excluded as is -- they go > > through access_remote_vm, starting with: > > if (mmap_read_lock_killable(mm)) > > return

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Linus Torvalds
On Fri, 4 Aug 2023 at 16:25, Mateusz Guzik wrote: > > I know of these guys, I think they are excluded as is -- they go > through access_remote_vm, starting with: > if (mmap_read_lock_killable(mm)) > return 0; > > while dup_mmap already write locks the parent's mm. Oh,

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Mateusz Guzik
On 8/5/23, Linus Torvalds wrote: > On Fri, 4 Aug 2023 at 14:46, Mateusz Guzik wrote: >> >> I don't see it mentioned in the discussion, so at a risk of ruffling >> feathers or looking really bad I'm going to ask: is the locking of any >> use if the forking process is single-threaded? T > > Sadly,

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Linus Torvalds
On Fri, 4 Aug 2023 at 14:46, Mateusz Guzik wrote: > > I don't see it mentioned in the discussion, so at a risk of ruffling > feathers or looking really bad I'm going to ask: is the locking of any > use if the forking process is single-threaded? T Sadly, we've always been able to access the mm

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-08-04 Thread Mateusz Guzik
On Sat, Jul 08, 2023 at 12:12:12PM -0700, Suren Baghdasaryan wrote: [..] > Lock VMAs of the parent process when forking a child, which prevents > concurrent page faults during fork operation and avoids this issue. > This fix can potentially regress some fork-heavy workloads. Kernel build > time

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-07-08 Thread Suren Baghdasaryan
On Sat, Jul 8, 2023 at 3:54 PM Linus Torvalds wrote: > > On Sat, 8 Jul 2023 at 15:36, Suren Baghdasaryan wrote: > > > > On Sat, Jul 8, 2023 at 2:18 PM Linus Torvalds > > > > > > Again - maybe I messed up, but it really feels like the missing > > > vma_start_write() was more fundamental, and not

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-07-08 Thread Linus Torvalds
On Sat, 8 Jul 2023 at 15:36, Suren Baghdasaryan wrote: > > On Sat, Jul 8, 2023 at 2:18 PM Linus Torvalds > > > > Again - maybe I messed up, but it really feels like the missing > > vma_start_write() was more fundamental, and not some "TLB coherency" > > issue. > > Sounds plausible. I'll try to

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-07-08 Thread Suren Baghdasaryan
On Sat, Jul 8, 2023 at 2:18 PM Linus Torvalds wrote: > > On Sat, 8 Jul 2023 at 12:12, Suren Baghdasaryan wrote: > > > > kernel/fork.c | 1 + > > 1 file changed, 1 insertion(+) > > I ended up editing your explanation a lot. > > I'm not convinced that the bug has much to do with the delayed tlb

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-07-08 Thread Linus Torvalds
On Sat, 8 Jul 2023 at 12:12, Suren Baghdasaryan wrote: > > kernel/fork.c | 1 + > 1 file changed, 1 insertion(+) I ended up editing your explanation a lot. I'm not convinced that the bug has much to do with the delayed tlb flushing. I think it's more fundamental than some tlb coherence issue:

Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

2023-07-08 Thread Suren Baghdasaryan
On Sat, Jul 8, 2023 at 12:12 PM Suren Baghdasaryan wrote: > > When forking a child process, parent write-protects an anonymous page > and COW-shares it with the child being forked using copy_present_pte(). > Parent's TLB is flushed right before we drop the parent's mmap_lock in > dup_mmap(). If