On Tue, Sep 27, 2022 at 06:38:05PM +0000, Nadav Amit wrote: > I only skimmed the patches that you sent. The last couple of patches seem a > bit rough and dirty, so I am sorry to say that I skipped them (too many > “TODO” and “XXX” for my taste). > > I am sure other will have better feedback than me. I understand there is a > tradeoff and that this mechanism is mostly for high performance > snapshotting/forking. It would be beneficial to see whether this mechanism > can somehow be combined with existing ones (mshare?). Still thanks for your feedback. :) I'm looking at the PTE refcount and mshare patches. And, maybe it can combine with them in the future. > The code itself can be improved. I found the reasoning about synchronization > and TLB flushes and synchronizations to be lacking, and the code to seem > potentially incorrect. Better comments would help, even if the code is > correct. > > There are additional general questions. For instance, when sharing a > page-table, do you properly update the refcount/mapcount of the mapped > pages? And are there any possible interactions with THP? Since access to those mapped pages will cost a lot of time, and this will make fork() even have more overhead. It will not update the refcount/mapcount of the mapped pages. I'm not familiar with THP right now. But we have a plan for looking at it to see what will happen with COW PTE. Currently, I can only say that I prefer to avoid involving the behavior of huge-page/THP. If there are any ideas here please tell us. Thanks, Chih-En Lin
