Date:   Tue, 21 Feb 2023 15:26:33 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Tejun Heo <tj@kernel.org>
Cc:     Michal Hocko <mhocko@suse.com>,
        Yosry Ahmed <yosryahmed@google.com>,
        Alistair Popple <apopple@nvidia.com>, linux-mm@kvack.org,
        cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
        jhubbard@nvidia.com, tjmercier@google.com, hannes@cmpxchg.org,
        surenb@google.com, mkoutny@suse.com, daniel@ffwll.ch,
        "Daniel P . Berrange" <berrange@redhat.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Zefan Li <lizefan.x@bytedance.com>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 14/19] mm: Introduce a cgroup for pinned memory
Message-ID: <Y/Ua6VcNe/DFh7X4@nvidia.com>
References: <Y+GcJQRhvjqFaaSp@mtj.duckdns.org>
 <Y+0rxoM4w9nilUMZ@dhcp22.suse.cz>
 <Y+0tWZxMUx/NZ3Ne@nvidia.com>
 <Y+3jcw9vo4ml5p0M@dhcp22.suse.cz>
 <Y+4lcq4Fge27TQIn@nvidia.com>
 <Y/T2pNQ70eMAl1sX@slm.duckdns.org>
 <Y/T+pw25oGmKqz1k@nvidia.com>
 <Y/T/bkcYc9Krw4rE@slm.duckdns.org>
 <Y/UEkNn0O65Pfi4e@nvidia.com>
 <Y/UIURDjR9pv+gzx@slm.duckdns.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y/UIURDjR9pv+gzx@slm.duckdns.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?V1wM8VrubHxKqHUQYvjXFfvTUnnmpjgBy35HRhaVru0yNqT1g8j6IH0ghOl/?=
 =?us-ascii?Q?oM937/JnMPdBYCnSBeRRpXO8Hsq4rXhy7Z0D2J4rtlFe9ynNdwztLb3wGRnw?=
 =?us-ascii?Q?Ad+GBlT8k35RQhAmNKLVzkjILeQG1nx3dY4f1zLGivfWUibPYOq4/gBRDXue?=
 =?us-ascii?Q?iXnY3TdMwwqZhQkMW2Myre+btpOo0yKQ7JEgZtmz7HUU9bKTtjohJ8weNzxg?=
 =?us-ascii?Q?FhoVdVmxtNoH4dDKCuropBZrGbyYiXOnLx40RNKO1t8RpuvEz27Ee4mBSz5+?=
 =?us-ascii?Q?hyjoNuLqAb1Xeh0JAiffKEG+R1f5HZa1szLGVSZb1xzJT3X6qkkzW8DWvj9i?=
 =?us-ascii?Q?xejdgqAIcFOhdb0LPr4VnuoG2xiduTJvvhbw2WIIec/wUqMApC0DeuTaTe1N?=
 =?us-ascii?Q?NDyWe9s07Sp3O4XMG2etoAv0h/s8eYDngBhIqQT9CYAM29JhmVLzjszfFvEX?=
 =?us-ascii?Q?KURLzBYY8MfFt6CTIFmADRj+vZ4dPbWdxKIRNpDSTnU5zQWNht2Dt8yDeqe7?=
 =?us-ascii?Q?Z2If+8qVlv9Mn93tgSzrnkyCOaD1NsJWzLD73j7d5TItom5PY0qcjcOgR3qU?=
 =?us-ascii?Q?cw4PZ/22jJNOIMFKucu2rBXOrZ8NKBP2qNarLtIvWFDfNnXkLh8ZUzc9hdMP?=
 =?us-ascii?Q?OaQ2Aw+SiUYOlE1tq1T2Y31B9iWP1mH1gbo2z6PDFkvZYEu0tscgbzkM2qAf?=
 =?us-ascii?Q?bAXZIY5IETiqJoxHpO3b6/Gvi/5yD0l+uD5V+kPj/2Cd6vuH8OV2ee1LLFE7?=
 =?us-ascii?Q?mgo77fBpFkiUxdKTQfOnYBZ+w9PZNnuEgJP5XsHdFqB1VkDIQrd18pJRAypN?=
 =?us-ascii?Q?eLUTQ3LnKfqFLeGG9GVqnFXy+2uuI/lOpPRiiexA7jkOgSb8COTbdMRsi5/y?=
 =?us-ascii?Q?HZku1P6LKcui9FMTx6ApMWa0uZhpCzgMALoBK+8SAZddMsbbKlMTB7NFIcQZ?=
 =?us-ascii?Q?0LiVtZm+Fv3Y6/+rb5kwshRu8fW6r9mHu74P8GiNEPUpjZYpmVE/QRb9C3xx?=
 =?us-ascii?Q?AaBsuF2jnJLtGD538RlPJMApZnBXqb9b6ITXsDDIOoMHjJkWup0C33bUfSdL?=
 =?us-ascii?Q?2NQuewddcegnv4vfF7MyZdWwXzZA88KWhlQC6zSXM05zdbePqODfIIiG0Mvf?=
 =?us-ascii?Q?woALinUsmNVhpbP5b0oxOEMT/zDsKgJiu1vgEEVu9vaRUs2W1UQz/R7WQySx?=
 =?us-ascii?Q?GooIt9lUg7FOPUNskpFpJHF0fNfSsSa14DmwUBIq/irlWEHsiFJLS3eTHxFP?=
 =?us-ascii?Q?IZzy+3kqkFkMK4ZfD6F2mjvo48/d2Yn7kyrSw99n1KP9zhRrBLndtMVW82GM?=
 =?us-ascii?Q?AvfDdxFWFkAArNF1r2e/IStq+Y9MsoeoYiqg3ZPG2J9sb1natCH/4hCxVA85?=
 =?us-ascii?Q?nG5S9APS5zEv+uF0YySWa9HwLhulSdzWE3pbtysfXsILboS0WiuEzw1H1ELG?=
 =?us-ascii?Q?jqgEI9kvNKh2omri6N9dpuXQTZFC9bgIRpWp0ptDnI3Y1Vo3VUEBGiFRxTIv?=
 =?us-ascii?Q?70Y8Jgz5D0zgry9+iOqSaRmVBHMhjDYGMSkjYoZ75DJuhVB2INKS6F+OcrED?=
 =?us-ascii?Q?xUVrerEzD9smz4uhMOSxSdh7dXk2fcI5+4Z718lE?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 30550aa4-da80-4f47-bc16-08db14418e3f
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Feb 2023 19:26:39.6903
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: RH3uxQ3UR/Gi1fj071+f/4LMBRbGcmGKqji0EB3mhXvfJLj7FxcZc52OU51qbluT
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7725
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Feb 21, 2023 at 08:07:13AM -1000, Tejun Heo wrote:
> > AFAIK there are few real use cases to establish a pin on MAP_SHARED
> > mappings outside your cgroup. However, it is possible, the APIs allow
> > it, and for security sandbox purposes we can't allow a process inside
> > a cgroup to triger a charge on a different cgroup. That breaks the
> > sandbox goal.
> 
> It seems broken anyway. Please consider the following scenario:

Yes, this is broken like this already today - memcg doesn't work
entirely perfectly for MAP_SHARED scenarios, IMHO.

> > > for whatever reason is determining the pinning ownership or should the page
> > > ownership be attributed the same way too? If they indeed need to differ,
> > > that probably would need pretty strong justifications.
> > 
> > It is inherent to how pin_user_pages() works. It is an API that
> > establishs pins on existing pages. There is nothing about it that says
> > who the page's memcg owner is.
> > 
> > I don't think we can do anything about this without breaking things.
> 
> That's a discrepancy in an internal interface and we don't wanna codify
> something like that into userspace interface. Semantially, it seems like if
> pin_user_pages() wanna charge pinning to the cgroup associated with an fd
> (or whatever), it should also claim the ownership of the pages
> themselves.

Multiple cgroup can pin the same page, so it is not as simple as just
transfering ownership, we need multi-ownership and to really fix the
memcg limitations with MAP_SHARED without an API impact.

You are right that pinning is really just a special case of
allocation, but there is a reason the memcg was left with weak support
for MAP_SHARED and changing that may be more than just hard but an
infeasible trade off..

At least I don't have a good idea how to even approach building a
reasonable datstructure that can track the number of
charges per-cgroup per page. :\

Jason