Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Suren Baghdasaryan]

On Tue, Jul 7, 2026 at 9:52 AM Lorenzo Stoakes <ljs@xxxxxxxxxx> wrote:
>
> +cc Vlata, the only one from the MEMORY MAPPING - LOCKING people excluded,
> and Willy as he's been heavily involved. But see below you need to cc- way
> more.
>
> (nitty I know) Maybe subject should be 'mm: default to VMA lock on
> VM_FAULT_RETRY' or something just to make it clear it's about the retry
> path?
>
> Also I guess you and Barry agreed to you taking this over? :)
>
> You missed a million people from cc- by the way. This time I really cannot
> be bothered to add them all as I usually do, but it is MANDATORY to cc
> maintainers and really you should be cc'ing reviewers too every time.
>
> Again I feel I need to write a bot that just autoreplies about this :)
>
> $ scripts/get_maintainer.pl --nogit --nogit-fallback your-patch.patch
> <... ~36 people in total ...>
> < lists >
> < etc. >
>
> Please fix this on respin.
>
> On Fri, Jun 26, 2026 at 03:50:19PM +0800, Hongru Zhang wrote:
> > From: Hongru Zhang <zhanghongru@xxxxxxxxxx>
> >
> > Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
> > lock due to folio_lock() failing, mainly because the folio is under
> > I/O, the architecture fault handler unconditionally falls back to
> > retrying with mmap_lock. This leads to mmap_lock contention.
> >
> > This patch introduces VM_FAULT_RETRY_HARD to mark the paths that
>
> I'm going to have to bikeshed the name :)
>
> While I like the idea of 'retry hard' like the movie 'die hard' :P I think,
> since this is explicitly about the mmap lock, we should reference that?
>
> So maybe VM_FAULT_RETRY_MMAP?
>
> or VM_FAULT_RETRY_HARDER_WITH_A_VENGEANCE? ;)
>
> > require mmap_lock. Retries now stay under the per-VMA lock by default,
> > and only marked paths fall back to the mmap_lock retry path.
>
> I think the commit message should be much clearer about the logic here.
>
> You are making it such that VM_FAULT_RETRY does its retry pass using a VMA
> lock, but it retains the same 'retry and set FAULT_FLAG_TRIED' logic as
> before.
>
> HOWEVER.
>
> On the VMA lock path the retry flag is only set if VM_FAULT_MAJOR. So now
> you're allowing _unbounded_ VMA lock attempts (which is concerning and I
> don't think intended by the lock code).
>
> None of this is clear right now, you need to documment it.
>
> >
> > Based on the stress model from Kunwu Chan and Wang Lian in v2, we
> > adapted a benchmark for a 20-core desktop environment (reduced thread
> > count, adjusted memcg limits, 20-core Intel i7-12700). The benchmark
> > uses concurrent page faults under memcg pressure (forcing reclaim and
> > VM_FAULT_RETRY) with parallel munmap to amplify mmap_lock read-write
> > contention.
> >
> > Throughput (higher is better):
> > +---------+------------+------------+-------------+
> > | Threads |  Vanilla   |  Patched   | Improvement |
> > +---------+------------+------------+-------------+
> > |   40    | 1071.11 /s | 1301.11 /s |   +21.5%    |
> > +---------+------------+------------+-------------+
> > |   60    | 1043.15 /s | 1472.36 /s |   +41.1%    |
> > +---------+------------+------------+-------------+
> > |   80    | 1049.39 /s | 1665.65 /s |   +58.7%    |
> > +---------+------------+------------+-------------+
> >
> > mmap_lock contention count (lower is better):
> > +---------+-----------+---------+-----------+
> > | Threads |  Vanilla  | Patched | Reduction |
> > +---------+-----------+---------+-----------+
> > |   40    | 3,217,904 |  51,201 |  -98.4%   |
> > +---------+-----------+---------+-----------+
> > |   60    | 4,419,149 |  55,711 |  -98.7%   |
> > +---------+-----------+---------+-----------+
> > |   80    | 5,395,730 |  66,184 |  -98.8%   |
> > +---------+-----------+---------+-----------+
>
> Yeah I worry a bit about the unbounded nature of things here, the fact
> we're seeing such a difference could also be offset by extreme busy waiting
> with the unbounded VMA lock retries.
>
> Let's experiment based on limiting this surely?
>
> >
> > Benchmark and test scripts:
> > https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
> >
> > Suggested-by: Barry Song <baohua@xxxxxxxxxx>
> > Suggested-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> > Signed-off-by: Hongru Zhang <zhanghongru@xxxxxxxxxx>
> > ---
> > Changes since v2:
> > - v2 required each retry path to explicitly opt in to VMA lock retry
> >   (VM_FAULT_RETRY_VMA). This patch inverts the logic: retries default
> >   to VMA lock, and only paths requiring mmap_lock opt out
> >   (VM_FAULT_RETRY_HARD)
> > - This patch corresponds to v2 patch 1/5; the remaining optimizations
> >   will be submitted separately
> > - Rebased on mm-unstable
> >
> > Link to v2:
> > https://lore.kernel.org/all/20260430040427.4672-1-baohua@xxxxxxxxxx/
> >
> > Link to v1:
> > https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@xxxxxxxxx/
>
> Hmm it's a bit odd for you to send an independent series and link to
> somebody else's series as v1/v2 but anyway as long as Barry and you agree
> with that approach that's fine!
>
> >
> >  arch/arm/mm/fault.c       | 4 ++++
> >  arch/arm64/mm/fault.c     | 4 ++++
> >  arch/loongarch/mm/fault.c | 4 ++++
> >  arch/powerpc/mm/fault.c   | 4 ++++
> >  arch/riscv/mm/fault.c     | 4 ++++
> >  arch/s390/mm/fault.c      | 4 ++++
> >  arch/x86/mm/fault.c       | 4 ++++
> >  include/linux/mm_types.h  | 9 +++++----
> >  mm/huge_memory.c          | 2 +-
> >  mm/memory.c               | 6 +++---
> >  10 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> > index e62cc4be5adf..2f909fb4f7db 100644
> > --- a/arch/arm/mm/fault.c
> > +++ b/arch/arm/mm/fault.c
> > @@ -391,6 +391,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -420,6 +421,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> >                       goto no_context;
> >               return 0;
> >       }
>
> OK so this is really important - and it seems to be the case regardless of
> arch.
>
> When VM_FAULT_RETRY is encountered under the VMA lock, we do:
>
>         if (fault & VM_FAULT_MAJOR)
>                 flags |= FAULT_FLAG_TRIED;
>
> Which then allows _under the mmap lock_ 2 more retries.
>
> You're now (silently!) going to:
>
>         if (!(fault & VM_FAULT_MAJOR))
>                 /* unbounded infinite VMA lock retries */
>
> This seems very unwise (TM).
>
> We should bound this somehow. A maximum number of retries? Only do the
> minor fault thing once?
>
> Could you benchmark how it looks at different retry counts? We could also
> consider other strategies, but unbounded attempts here seems not good.
>
> Another thing - is there any way to dedupe this code...? We're open coding
> a lot of crap now in each of the arches and it's not good.
>
> A series like this is the place to fix stuff like that, not keep adding to
> the technical debt.
>
> Hongru - could you see if that's possible as preceeding patches please?
>
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
>
> Maybe it's worth being explicit that this is the VM_FAULT_RETRY path? Otherwise
> it's not so clear.
>
> You happen to only fall through here if VM_FAULT_RETRY is set.
>
> So e.g.:
>
> /**
>  * fault_can_retry_vma_lock() - Are we able to retry a page fault holding
>  * only the VMA lock?
>  * @fault: The fault to check.
>  *
>  * We default to performing the VM_FAULT_RETRY/FAULT_FLAG_TRIED dance under
>  * VMA lock. This allows us to avoid mmap contention on the likely case
>  * that we encountered an ephemeral inability to VMA lock (e.g. the VMA was
>  * write locked at the time).
>  *
>  * Returns: true if we can retry under the VMA lock, false otherwise.
>  */
> static bool fault_can_retry_vma_lock(vm_fault_t fault)
> {
>         VM_WARN_ON_ONCE(!(fault & VM_FAULT_RETRY));
>
>         return fault & VM_FAULT_RETRY_HARD;
> }
>
> And put that somewhere all the arch fault logic can access.
>
> Then:
>
>         if (fault_can_retry_vma_lock(fault))
>                 goto retry_vma;
>
> Is self-documenting, and the assert assures that no mistake was made with
> it.
>
> Same comments for all the arches, obviously.
>
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > index 739800835920..695a09486795 100644
> > --- a/arch/arm64/mm/fault.c
> > +++ b/arch/arm64/mm/fault.c
> > @@ -673,6 +673,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> >       if (!(mm_flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -719,6 +720,9 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> >                       goto no_context;
> >               return 0;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
> > index 2c93d33356e5..d7dd55722e52 100644
> > --- a/arch/loongarch/mm/fault.c
> > +++ b/arch/loongarch/mm/fault.c
> > @@ -219,6 +219,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -265,6 +266,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> >                       no_context(regs, write, address);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index 806c74e0d5ab..65ec9a8252e7 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -487,6 +487,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -517,6 +518,9 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> >       if (fault_signal_pending(fault, regs))
> >               return user_mode(regs) ? 0 : SIGBUS;
> >
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >       /* When running in the kernel we expect faults to occur only to
> > diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> > index 04ed6f8acae4..6c68414a1224 100644
> > --- a/arch/riscv/mm/fault.c
> > +++ b/arch/riscv/mm/fault.c
> > @@ -347,6 +347,7 @@ void handle_page_fault(struct pt_regs *regs)
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -376,6 +377,9 @@ void handle_page_fault(struct pt_regs *regs)
> >                       no_context(regs, addr);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> > index 028aeb9c48d6..45a6c35044cc 100644
> > --- a/arch/s390/mm/fault.c
> > +++ b/arch/s390/mm/fault.c
> > @@ -294,6 +294,7 @@ static void do_exception(struct pt_regs *regs, int access)
> >               flags |= FAULT_FLAG_WRITE;
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -318,6 +319,9 @@ static void do_exception(struct pt_regs *regs, int access)
> >                       handle_fault_error_nolock(regs, 0);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >  retry:
> >       vma = lock_mm_and_find_vma(mm, address, regs);
> > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> > index 63de8e8684f2..cb80070271d0 100644
> > --- a/arch/x86/mm/fault.c
> > +++ b/arch/x86/mm/fault.c
> > @@ -1322,6 +1322,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -1351,6 +1352,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> >                                                ARCH_DEFAULT_PKEY);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ef78617ce93..58db6f0af6fd 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
>
> Please also update the comment block to add the new flag.
>
> > @@ -1665,10 +1665,11 @@ enum vm_fault_reason {
> >       VM_FAULT_NOPAGE         = (__force vm_fault_t)0x000100,
> >       VM_FAULT_LOCKED         = (__force vm_fault_t)0x000200,
> >       VM_FAULT_RETRY          = (__force vm_fault_t)0x000400,
> > -     VM_FAULT_FALLBACK       = (__force vm_fault_t)0x000800,
> > -     VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000,
> > -     VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000,
> > -     VM_FAULT_COMPLETED      = (__force vm_fault_t)0x004000,
> > +     VM_FAULT_RETRY_HARD     = (__force vm_fault_t)0x000800,
> > +     VM_FAULT_FALLBACK       = (__force vm_fault_t)0x001000,
> > +     VM_FAULT_DONE_COW       = (__force vm_fault_t)0x002000,
> > +     VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x004000,
> > +     VM_FAULT_COMPLETED      = (__force vm_fault_t)0x008000,
>
> Hmm I suppose there's no harm in renumbering here to keep the retries
> together :)
>
> I wonder if we shouldn't invert this VM_FAULT_RETRY | VM_FAULT_RETRY_HARD
> thing though and instead have:
>
> #define VM_FAULT_RETRY_MASK (VM_FAULT_RETRY | VM_FAULT_RETRY_MMAP)
>
> And convert code that checks VM_FAULT_RETRY to check VM_FAULT_RETRY_MASK
> instead, then there'd be no need for an awkward check/or confusion around
> the need for VM_FAULT_RETRY to always be set in both cases.
>
> You're already basically touching all the files that reference
> VM_FAULT_RETRY so it's not actually much more churn.
>
> >       VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
> >  };
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 94bd656eeaf8..ad0129d579bc 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1398,7 +1398,7 @@ vm_fault_t do_huge_pmd_device_private(struct vm_fault *vmf)
> >
> >       if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> >               vma_end_read(vma);
> > -             return VM_FAULT_RETRY;
> > +             return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
>
> Why are we specifically wanting this for PMD device private entries?
>
> You should add a comment, and the commit message should say why.
>
> Actually, maybe in this common case of vmf->flags & FAULT_FLAG_VMA_LOCK ->
> harder you can make a helper?
>
> Like:
>
> /** blahhhh some kdoc comment */
> static vm_fault_t retry_under_mmap_if_vma_locked(struct vm_fault *vmf)
> {
>         if (!(vmf->flags & FAULT_FLAG_VMA_LOCK))
>                 return 0;
>
>         vma_end_read(vma);
>         return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
> }
>
> And:
>
>         vm_fault_t ret;
>
>         ret = retry_under_mmap_if_vma_locked(vmf);
>         if (ret)
>                 return ret;
>
> Or something like this.
>
> >       }
> >
> >       ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index ff338c2abe92..7f2a30b5efca 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3797,7 +3797,7 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
> >       if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK))
> >               return 0;
> >       vma_end_read(vma);
> > -     return VM_FAULT_RETRY;
> > +     return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
>
> You could use my helper like above.
>
> >  }
> >
> >  /**
> > @@ -3824,7 +3824,7 @@ vm_fault_t __vmf_anon_prepare(struct vm_fault *vmf)
> >               return 0;
> >       if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
> >               if (!mmap_read_trylock(vma->vm_mm))
> > -                     return VM_FAULT_RETRY;
> > +                     return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
>
> This really makes me think VM_FAULT_RETRY_MMAP or something explicit would
> be better, otherwise you have to add a comment explaining you're doing this
> because you need the mmap read lock here since the VMA lock + speculative
> mmap read lock acquisition didn't work out.
>
> >       }
> >       if (__anon_vma_prepare(vma))
> >               ret = VM_FAULT_OOM;
> > @@ -4778,7 +4778,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> >                                * under VMA lock.
> >                                */
> >                               vma_end_read(vma);
> > -                             ret = VM_FAULT_RETRY;
> > +                             ret = VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
>
> Again, helper above self-documents.
>
> >                               goto out;
> >                       }
> >
> >
> > base-commit: 81652c5a65d4ae28e9b18c16ef917a40025c3653
> > --
> > 2.43.0
> >
>
> Have you audited every single VM_FAULT_RETRY case? Because getting this
> wrong could be a serious problem and break things. You should include a
> list of call sites in your patch and explain why it's fine in each case.

That's my main worry with this patch as well. I think a better
approach would keep VM_FAULT_RETRY as is (falling back to mmap_lock),
introduce VM_FAULT_RETRY_VMA (or whatever is best to indicate a retry
with VMA lock) and then convert individual VM_FAULT_RETRY cases to
VM_FAULT_RETRY_VMA with proper reasoning and explanation why it is
safe. Less churn, gradual conversion, and happy reviewers.

>
> One case I noticed is uffd:
>
>         ret = VM_FAULT_RETRY;
>         ...
>         if (unlikely(READ_ONCE(ctx->released))) {
>                 /*
>                  * If a concurrent release is detected, do not return
>                  * VM_FAULT_SIGBUS or VM_FAULT_NOPAGE, but instead always
>                  * return VM_FAULT_RETRY with lock released proactively.
> .       ...
>                  * If we were to return VM_FAULT_NOPAGE, it would work for
>                  * the fault path, in which the lock will be released
>                  * later.  However for GUP, faultin_page() does nothing
>                  * special on NOPAGE, so GUP would spin retrying without
>                  * releasing the mmap read lock, causing possible livelock.
>                  *
>                  * Here only VM_FAULT_RETRY would make sure the mmap lock
>                  * be released immediately, so that the thread concurrently
>                  * releasing the userfault would always make progress.
>                  */
>                  release_fault_lock(vmf);
>                  goto out;
>         }
>
> The comment needs updating (Suren?) to reflect the VMA lock, basically with
> VMA lock held this is meaningless.

Right, I missed it when enabling per-VMA locks for userfaultfd faults
in [1]. The comment is true if we already fell back to mmap_lock but
not if we are handling the fault under VMA lock. I should amend it.

[1] https://lore.kernel.org/all/20230630211957.1341547-1-surenb@xxxxxxxxxx/

>
> We're actually safe here in general as if the VMA lock is contended on a
> competing write lock we drop to mmap lock.
>
> A racing uffd release thread does an unconditional vma_start_write()
> _after_ setting ctx->released:
>
> static int userfaultfd_release(struct inode *inode, struct file *file)
> {
>         ...
>         WRITE_ONCE(ctx->released, true);
>         userfaultfd_release_all(mm, ctx);
>         ...
> }
>
> userfaultfd_release_all()
> <mmap write lock>
> -> userfaultfd_clear_vma()
> -> vma_modify_flags_uffd()
> -> vma_modify()
> -> vma_merge_existing_range()   -> vma_start_write() LIVELOCK
> OR split_vma() -> __split_vma() -> vma_start_write() LIVELOCK
>
> So yeah, not sure on this one, maybe we're fine just to use VMA lock still?
>
> Suren - any thoughts?

If we retry without falling back to mmap_lock then
userfaultfd_release() can end up write-locking mmap_lock but losing
the race for VMA lock to the retrying pagefault handler. This means it
would block while holding mmap_lock for write - not good.
With pagefault handler falling back to mmap_lock, either
userfaultfd_release() wins the race for mmap_lock or pagefault
handler, so I think we are safe.

>
> You definitely need to explain your reasoning in the commit message at
> least.
>
> Also commit 29a22b9e08d7 ("mm: handle userfaults under VMA lock") did fail
> to update this comment to reflect VMA locks - we should fix that too -
> Suren? :)

Yes, I will correct my mistake and send a small fixup.

>
>
> Also another thought - should we allow FAULT_FLAG_RETRY_NOWAIT to retry
> with the VMA lock? As this could also be some unbounded spinning? But then
> again, if we bound the spinning that might be ok.

Hmm. If we do that, let's make it a separate series. Too many moving
parts is never a good idea.

>
> Thanks, Lorenzo