Date: Wed, 25 Jun 2025 06:05:21 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Christoph Berg <myon@debian.org>
Cc: Tomas Vondra <tomas@vondra.me>, Andres Freund <andres@anarazel.de>,
	Tomas Vondra <tomas.vondra@postgresql.org>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Introduce pg_shmem_allocations_numa view
Message-ID: <aFuRoUieUVh+pMfZ@ip-10-97-1-34.eu-west-3.compute.internal>
References: <0643ae61-cf9d-482c-9b2c-fb861b24fd22@vondra.me>
 <aFnGQ2lfb8cfukim@msg.df7cb.de>
 <cd16c4a3-2860-46de-a74e-5532d1b2ee39@vondra.me>
 <6342f601-77de-4ee0-8c2a-3deb50ceac5b@vondra.me>
 <aFpg1de9ZfS1QgUt@ip-10-97-1-34.eu-west-3.compute.internal>
 <a3a4fe3d-1a80-4e03-aa8e-150ee15f6c35@vondra.me>
 <aFqHoNXQ/uWKXJ4U@ip-10-97-1-34.eu-west-3.compute.internal>
 <8649a4e3-c60d-4f37-aa6f-e7e7c14c581e@vondra.me>
 <aFqnM/iiL+MB62dG@ip-10-97-1-34.eu-west-3.compute.internal>
 <aFq5HQj016rVS2lm@msg.df7cb.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="iGQ7zMJ2Rzk0wyeD"
Content-Disposition: inline
In-Reply-To: <aFq5HQj016rVS2lm@msg.df7cb.de>
Archived-At: 
 <https://www.postgresql.org/message-id/aFuRoUieUVh%2BpMfZ%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk


--iGQ7zMJ2Rzk0wyeD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

On Tue, Jun 24, 2025 at 04:41:33PM +0200, Christoph Berg wrote:
> Re: Bertrand Drouvot
> > Yes, I think compat_uptr_t usage is missing in do_pages_stat() (while it's used
> > in do_pages_move()).
> 
> I was also reading the kernel source around that place but you spotted
> the problem before me. This patch resolves the issue here:
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 8cf0f9c9599..542c81ec3ed 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2444,7 +2444,13 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>  		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
>  			break;
> 
> -		pages += chunk_nr;
> +		if (in_compat_syscall()) {
> +			compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
> +
> +			pages32 += chunk_nr;
> +			pages = (const void __user * __user *) pages32;
> +		} else
> +			pages += chunk_nr;
>  		status += chunk_nr;
>  		nr_pages -= chunk_nr;
>  	}
> 

Thanks! Yeah, I had the same kind of patch idea in mind.

> > +               #define NUMA_QUERY_CHUNK_SIZE 16  /* has to be <= DO_PAGES_STAT_CHUNK_NR (do_pages_stat())*/
> > +
> > +               for (uint64 chunk_start = 0; chunk_start < shm_ent_page_count; chunk_start += NUMA_QUERY_CHUNK_SIZE) {
> 
> Perhaps optimize it to this:
> 
> #if sizeof(void *) == 4
> #define NUMA_QUERY_CHUNK_SIZE 16  /* has to be <= DO_PAGES_STAT_CHUNK_NR (do_pages_stat())*/
> #else
> #define NUMA_QUERY_CHUNK_SIZE 1024
> #endif
> 
> ... or some other bigger number.

I had in mind to split the batch size on the PG side only for 32-bits, what about
the attached?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--iGQ7zMJ2Rzk0wyeD
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-Work-around-Linux-kernel-bug-in-do_pages_stat.patch"