On 2023/08/03 23:49, Michal Hocko wrote: > On Thu 03-08-23 22:18:10, Tetsuo Handa wrote: >> On 2023/07/31 23:25, Michal Hocko wrote: >>> On Sat 29-07-23 20:05:43, Tetsuo Handa wrote: >>>> On 2023/07/29 14:31, Tetsuo Handa wrote: >>>>> On 2023/07/28 0:10, Sebastian Andrzej Siewior wrote: >>>>>> On 2023-06-28 21:14:16 [+0900], Tetsuo Handa wrote: >>>>>>>> Anyway, please do not do this change only because of printk(). >>>>>>>> IMHO, the current ordering is more logical and the printk() problem >>>>>>>> should be solved another way. >>>>>>> >>>>>>> Then, since [PATCH 1/2] cannot be applied, [PATCH 2/2] is automatically >>>>>>> rejected. >>>>>> >>>>>> My understanding is that this patch gets applied and your objection will >>>>>> be noted. >>>>> >>>>> My preference is that zonelist_update_seq is not checked by !__GFP_DIRECT_RECLAIM >>>>> allocations, which is a low-hanging fruit towards GFP_LOCKLESS mentioned at >>>>> https://lkml.kernel.org/r/ZG3+l4qcCWTPtSMD@xxxxxxxxxxxxxx and >>>>> https://lkml.kernel.org/r/ZJWWpGZMJIADQvRS@xxxxxxxxxxxxxx . >>>>> >>>>> Maybe we can defer checking zonelist_update_seq till retry check like below, >>>>> for this is really an infrequent event. >>>>> >>>> >>>> An updated version with comments added. >>> >>> Seriously, don't you see how hairy all this is? And for what? Nitpicking >>> something that doesn't seem to be a real problem in the first place? >> >> Seriously, can't you find "zonelist_update_seq is not checked by !__GFP_DIRECT_RECLAIM >> allocations, which is a low-hanging fruit towards GFP_LOCKLESS" !? > > I do not think we have concluded that we want to support GFP_LOCKLESS. > This might be trivial straightforward now but it imposes some constrains > for future maintainability. So far we haven't heard about many usecases > where this would be needed and a single one is not sufficient IMHO. When you introduced a word GFP_LOCKLESS in the link above, I was wondering the meaning of "LESS" part. Since we know that it is difficult to achieve "hold 0 lock during memory allocation", "hold least locks during memory allocation" will be at best. Therefore, GFP_LOCKLESS is as misleading name as GFP_ATOMIC. GFP_LOCK_LEAST or GFP_LEAST_LOCKS will represent the real behavior better. Like I said I consider that memory allocations which do not do direct reclaim should be geared towards less locking dependency. in the thread above, I still believe that this what-you-call-hairy version (which matches "hold least locks during memory allocation" direction) is better than "[PATCH v3 2/2] mm/page_alloc: Use write_seqlock_irqsave() instead write_seqlock() + local_irq_save()." (which does not match "hold least locks during memory allocation"). My version not only avoids possibility of deadlock, but also makes zonelist_iter_begin() faster and simpler. Not holding zonelist_update_seq lock is trivially doable compared to removing __GFP_KSWAPD_RECLAIM from GFP_ATOMIC. Please give me feedback about which line of my proposal is technically unsafe, instead of discarding my proposal with negative words like "hairy".
