Re: [PATCH v2 1/2] seqlock: Do the lockdep annotation before locking in do_write_seqcount_begin_nested()

Michal Hocko
2023-08-07T08:20:31Z

On Fri 04-08-23 22:27:22, Tetsuo Handa wrote:
> 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@dhcp22.suse.cz and
> >>>>> https://lkml.kernel.org/r/ZJWWpGZMJIADQvRS@dhcp22.suse.cz .
> >>>>>
> >>>>> 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.

I am not sure I understand what least locks mean actually. I guess what
you wanted to say is that there are no locks or other synchronization
means with external visibility/dependencies used. In other words a mode
which can be called from any locking context. That would be certainly
possible and whether any internal locks are used or not is an
implementation detail as long as the no external visibility/dependencies
rule is held. I do not really want to start the naming discussion as it
is not really clear we want/need to provide such a strong guarantee.
--
Michal Hocko
SUSE Labs