From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 41A22C433C1 for ; Tue, 23 Mar 2021 17:01:50 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 99B32619BB for ; Tue, 23 Mar 2021 17:01:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 99B32619BB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id EF34C6B0207; Tue, 23 Mar 2021 13:01:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EA3B86B020D; Tue, 23 Mar 2021 13:01:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D1CB56B020F; Tue, 23 Mar 2021 13:01:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0248.hostedemail.com [216.40.44.248]) by kanga.kvack.org (Postfix) with ESMTP id B2BFD6B0207 for ; Tue, 23 Mar 2021 13:01:48 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 6C34D180C6F11 for ; Tue, 23 Mar 2021 17:01:48 +0000 (UTC) X-FDA: 77951755896.28.BB98F78 Received: from mail-qt1-f171.google.com (mail-qt1-f171.google.com [209.85.160.171]) by imf18.hostedemail.com (Postfix) with ESMTP id 4ACD62000AF0 for ; Tue, 23 Mar 2021 17:01:38 +0000 (UTC) Received: by mail-qt1-f171.google.com with SMTP id m7so15412003qtq.11 for ; Tue, 23 Mar 2021 10:01:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=aMZf3nAnd3Ur++OR8V/tDE1PbcZI5ey7Mp8g1oFzdQ8=; b=hn+/QKTWQ8HIyPatT9wTlycQejd9EC8+QxrYdIXm2iCp2ZexLrV0+iBG3Z8i4zXusA 7/JD+um93ZmFKBY28q3Bna9RyArSGxILyxRJAmTuFK9RmO4VEbll8xungPHroHkZ/g/j Dth/GtDL7UnUJzXClUZQNYArZfATgy2XzyDoJHEtIDqTfq26v8tCj7FnIQJ/y7E3y0sn feoWvo6GCwYAJA4V2MXn8K80zZJdVdYqhxi7TJQv2SusDlupOFf0lMAhfNti9jNbB/ci zq7V0lrPsOlc3l7P5BJgApsv4RQ9/BQrTTg6woTtm6gpjiJJknRrdUwG+YCu7gQ1I6pg rYtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=aMZf3nAnd3Ur++OR8V/tDE1PbcZI5ey7Mp8g1oFzdQ8=; b=gT2gn3V/5tpW24vs3HhcUhJkCP2XDhhH9g/6Ad9V6EHu8G0GVRuFiektYy6tV7fqiN kiHaNfL/0mL2qMBDK3MSCrI1oWE3t3gww7NDwe/GTBy2vICM02Ae6ik3Lj68h2GHeS9e Su/jrNFMkWOxwZPn6ZN9ZgIC3Z0gMNf7kEh//VNd5GbgqDZxggC3PTwXv9fBRquINSrN m6be2DHPePF3VrjNyrYwumWxHjkfkXi2ZriqVuwUGy2yihivw4pAAsCqPmEAIWU6GXY9 bB4twO6c4rnCDJ/O7uiwttXvkj+DG/WUos78v3i4sjg21Uyjkp3HUsIHC5Y3rozRG7HO fVyA== X-Gm-Message-State: AOAM53223Lw2MVlkp1jK6dK1YlqvRa7dGcQ/Zga+2n+nYxjnF5gXLv0T NFRy26We5nCuPF02ijfhjYMAEw== X-Google-Smtp-Source: ABdhPJzCuLhCkYKHVasYPzgXQeDfWHGfRizwL/eEVZMvB/YhmsZ90glH0Z4eOlPyJJ67gc69/Mr1/w== X-Received: by 2002:a05:622a:1389:: with SMTP id o9mr5395247qtk.18.1616518897768; Tue, 23 Mar 2021 10:01:37 -0700 (PDT) Received: from localhost (70.44.39.90.res-cmts.bus.ptd.net. [70.44.39.90]) by smtp.gmail.com with ESMTPSA id i14sm5006234qtq.81.2021.03.23.10.01.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Mar 2021 10:01:37 -0700 (PDT) Date: Tue, 23 Mar 2021 13:01:36 -0400 From: Johannes Weiner To: Arjun Roy Cc: Arjun Roy , Andrew Morton , David Miller , netdev , Linux Kernel Mailing List , Cgroups , Linux MM , Shakeel Butt , Eric Dumazet , Soheil Hassas Yeganeh , Jakub Kicinski , Michal Hocko , Yang Shi , Roman Gushchin Subject: Re: [mm, net-next v2] mm: net: memcg accounting for TCP rx zerocopy Message-ID: References: <20210316041645.144249-1-arjunroy.kdev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: x9n9rk31yfsac17g4dz3nptqmra197j1 X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 4ACD62000AF0 Received-SPF: none (cmpxchg.org>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=mail-qt1-f171.google.com; client-ip=209.85.160.171 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616518898-897255 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Mar 22, 2021 at 02:35:11PM -0700, Arjun Roy wrote: > To make sure we're on the same page, then, here's a tentative > mechanism - I'd rather get buy in before spending too much time on > something that wouldn't pass muster afterwards. > > A) An opt-in mechanism, that a driver needs to explicitly support, in > order to get properly accounted receive zerocopy. Yep, opt-in makes sense. That allows piece-by-piece conversion and avoids us having to have a flag day. > B) Failure to opt-in (e.g. unchanged old driver) can either lead to > unaccounted zerocopy (ie. existing behaviour) or, optionally, > effectively disabled zerocopy (ie. any call to zerocopy will return > something like EINVAL) (perhaps controlled via some sysctl, which > either lets zerocopy through or not with/without accounting). I'd suggest letting it fail gracefully (i.e. no -EINVAL) to not disturb existing/working setups during the transition period. But the exact policy is easy to change later on if we change our minds on it. > The proposed mechanism would involve: > 1) Some way of marking a page as being allocated by a driver that has > decided to opt into this mechanism. Say, a page flag, or a memcg flag. Right. I would stress it should not be a memcg flag or any direct channel from the network to memcg, as this would limit its usefulness while having the same maintenance overhead. It should make the network page a first class MM citizen - like an LRU page or a slab page - which can be accounted and introspected as such, including from the memcg side. So definitely a page flag. > 2) A callback provided by the driver, that takes a struct page*, and > returns a boolean. The value of the boolean being true indicates that > any and all refs on the page are held by the driver. False means there > exists at least one reference that is not held by the driver. I was thinking the PageNetwork flag would cover this, but maybe I'm missing something? > 3) A branch in put_page() that, for pages marked thus, will consult > the driver callback and if it returns true, will uncharge the memcg > for the page. The way I picture it, put_page() (and release_pages) should do this: void __put_page(struct page *page) { if (is_zone_device_page(page)) { put_dev_pagemap(page->pgmap); /* * The page belongs to the device that created pgmap. Do * not return it to page allocator. */ return; } + + if (PageNetwork(page)) { + put_page_network(page); + /* Page belongs to the network stack, not the page allocator */ + return; + } if (unlikely(PageCompound(page))) __put_compound_page(page); else __put_single_page(page); } where put_page_network() is the network-side callback that uncharges the page. (..and later can be extended to do all kinds of things when informed that the page has been freed: update statistics (mod_page_state), put it on a private network freelist, or ClearPageNetwork() and give it back to the page allocator etc. But for starters it can set_page_count(page, 1) after the uncharge to retain the current silent recycling behavior.) > The anonymous struct you defined above is part of a union that I think > normally is one qword in length (well, could be more depending on the > typedefs I saw there) and I think that can be co-opted to provide the > driver callback - though, it might require growing the struct by one > more qword since there may be drivers like mlx5 that are already using > the field already in there for dma_addr. The page cache / anonymous struct it's shared with is 5 words (double linked list pointers, mapping, index, private), and the network struct is currently one word, so you can add 4 words to a PageNetwork() page without increasing the size of struct page. That should be plenty of space to store auxiliary data for drivers, right? > Anyways, the callback could then be used by the driver to handle the > other accounting quirks you mentioned, without needing to scan the > full pool. Right. > Of course there are corner cases and such to properly account for, but > I just wanted to provide a really rough sketch to see if this > (assuming it were properly implemented) was what you had in mind. If > so I can put together a v3 patch. Yeah, makes perfect sense. We can keep iterating like this any time you feel you accumulate too many open questions. Not just for MM but also for the networking folks - although I suspect that the first step would be mostly about the MM infrastructure, and I'm not sure how much they care about the internals there ;) > Per my response to Andrew earlier, this would make it even more > confusing whether this is to be applied against net-next or mm trees. > But that's a bridge to cross when we get to it. The mm tree includes -next, so it should be a safe development target for the time being. I would then decide it based on how many changes your patch interacts with on either side. Changes to struct page and the put path are not very frequent, so I suspect it'll be easy to rebase to net-next and route everything through there. And if there are heavy changes on both sides, the -mm tree is the better route anyway. Does that sound reasonable?