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=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL 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 215E4C433DB for ; Wed, 24 Mar 2021 20:55:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E10AB61A17 for ; Wed, 24 Mar 2021 20:55:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238296AbhCXUyA (ORCPT ); Wed, 24 Mar 2021 16:54:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49072 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238128AbhCXUxt (ORCPT ); Wed, 24 Mar 2021 16:53:49 -0400 Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E5C63C0613DE for ; Wed, 24 Mar 2021 13:53:47 -0700 (PDT) Received: by mail-lf1-x136.google.com with SMTP id n138so34032188lfa.3 for ; Wed, 24 Mar 2021 13:53:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9IAjIh7gf44rR4epDI0vFAr+lh0+8IoyQ48EbdutPC4=; b=X+i+XcHts81gTX2YQSoV3UFieH2kepMG7Wygr2n+8tt5NHgaQ+mstmTUI3J5ISoI7U SUl3tiJvsbwRVOLXnmugaf7VxQxxdhAWYfBnAG00WyqH9bu/e4yNA5k3oEymKHyTzkLh TTT+a8G71fKF6cc1A4Vkr/zD4HBlTVppNUf5AaI1iqxq87iXB7fbncGKeplOfgTH82ws hdeCH/aQeXDSvTbv1qPdy2Lh7zjA6lcDs1+f+DHw1q7iu3l5yphxkZpMqeDxCa8CTxyZ 5VTmYNZSiJct4WOQqCy68/ZoK5rI3L/qFEYbBPrXDsH1QW2R7QyRDf07jo8vRTObrEez ApCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9IAjIh7gf44rR4epDI0vFAr+lh0+8IoyQ48EbdutPC4=; b=KwEXZC9Nxn7csuezbhSxBb79MDOSmF8U9yP9gMlaCVaOe4BlFjGMuBP+Y7/d7RBVpi T82KtUERMpXxXilTIGuw6fBrJ4G+4ZwJd0gS+eMdgccE/IGQERwPT3VrqcnVGhtP9/Vh xEdkAwMf7Qkc0z07ytPhl69MUq0R2H23LTME7ejewSb9QT4xVFP4uiI5Lk120I+TfLqW UWXr1sVNsWwZPMrGi6ZbPuJ2GsqzL2c2+Xtn3AzzZP6tGN+ZEHK9mN6F2gWooHgw6dGz HfqSNG5Ac0SJSWyw0tEsQTtoKLEdVnFg39rdjcmyTGBDiXkenTLH4miVc25YNBMyJAb9 pv7g== X-Gm-Message-State: AOAM531NC/pqX3aMmixqyTqtFnQPu9gk7Y7+tVqSxsg8qjyOlzePfl3S Jf0Y2frg8F9pqMfQJ+V/lEfp4tqav3HESnu3eWPScg== X-Google-Smtp-Source: ABdhPJw0lQ/lHICaIcFDzikDJKy0JhvudtbKraeVHNse46qud1jmkyD20uZNubiTW6uoYiF8GjRukNZVynWF7Og7DJA= X-Received: by 2002:a19:c14a:: with SMTP id r71mr2932916lff.358.1616619225992; Wed, 24 Mar 2021 13:53:45 -0700 (PDT) MIME-Version: 1.0 References: <20210316041645.144249-1-arjunroy.kdev@gmail.com> In-Reply-To: From: Shakeel Butt Date: Wed, 24 Mar 2021 13:53:34 -0700 Message-ID: Subject: Re: [mm, net-next v2] mm: net: memcg accounting for TCP rx zerocopy To: Arjun Roy Cc: Michal Hocko , Johannes Weiner , Arjun Roy , Andrew Morton , David Miller , netdev , Linux Kernel Mailing List , Cgroups , Linux MM , Eric Dumazet , Soheil Hassas Yeganeh , Jakub Kicinski , Yang Shi , Roman Gushchin Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 24, 2021 at 1:39 PM Arjun Roy wrote: > > On Wed, Mar 24, 2021 at 2:12 AM Michal Hocko wrote: > > > > On Tue 23-03-21 11:47:54, Arjun Roy wrote: > > > On Tue, Mar 23, 2021 at 7:34 AM Michal Hocko wrote: > > > > > > > > On Wed 17-03-21 18:12:55, Johannes Weiner wrote: > > > > [...] > > > > > Here is an idea of how it could work: > > > > > > > > > > struct page already has > > > > > > > > > > struct { /* page_pool used by netstack */ > > > > > /** > > > > > * @dma_addr: might require a 64-bit value even on > > > > > * 32-bit architectures. > > > > > */ > > > > > dma_addr_t dma_addr; > > > > > }; > > > > > > > > > > and as you can see from its union neighbors, there is quite a bit more > > > > > room to store private data necessary for the page pool. > > > > > > > > > > When a page's refcount hits zero and it's a networking page, we can > > > > > feed it back to the page pool instead of the page allocator. > > > > > > > > > > From a first look, we should be able to use the PG_owner_priv_1 page > > > > > flag for network pages (see how this flag is overloaded, we can add a > > > > > PG_network alias). With this, we can identify the page in __put_page() > > > > > and __release_page(). These functions are already aware of different > > > > > types of pages and do their respective cleanup handling. We can > > > > > similarly make network a first-class citizen and hand pages back to > > > > > the network allocator from in there. > > > > > > > > For compound pages we have a concept of destructors. Maybe we can extend > > > > that for order-0 pages as well. The struct page is heavily packed and > > > > compound_dtor shares the storage without other metadata > > > > int pages; /* 16 4 */ > > > > unsigned char compound_dtor; /* 16 1 */ > > > > atomic_t hpage_pinned_refcount; /* 16 4 */ > > > > pgtable_t pmd_huge_pte; /* 16 8 */ > > > > void * zone_device_data; /* 16 8 */ > > > > > > > > But none of those should really require to be valid when a page is freed > > > > unless I am missing something. It would really require to check their > > > > users whether they can leave the state behind. But if we can establish a > > > > contract that compound_dtor can be always valid when a page is freed > > > > this would be really a nice and useful abstraction because you wouldn't > > > > have to care about the specific type of page. > > > > > > > > But maybe I am just overlooking the real complexity there. > > > > -- > > > > > > For now probably the easiest way is to have network pages be first > > > class with a specific flag as previously discussed and have concrete > > > handling for it, rather than trying to establish the contract across > > > page types. > > > > If you are going to claim a page flag then it would be much better to > > have it more generic. Flags are really scarce and if all you care about > > is PageHasDestructor() and provide one via page->dtor then the similar > > mechanism can be reused by somebody else. Or does anything prevent that? > > The way I see it - the fundamental want here is, for some arbitrary > page that we are dropping a reference on, to be able to tell that the > provenance of the page is some network driver's page pool. If we added > an enum target to compound_dtor, if we examine that offset in the page > and look at that value, what guarantee do we have that the page isn't > instead some other kind of page, and the byte value there was just > coincidentally the one we were looking for (but it wasn't a network > driver pool page)? > > Existing users of compound_dtor seem to check first that a > PageCompound() or PageHead() return true - the specific scenario here, > of receiving network packets, those pages will tend to not be compound > (and more specifically, compound pages are explicitly disallowed for > TCP receive zerocopy). > > Given that's the case, the options seem to be: > 1) Use a page flag - with the downside that they are a severely > limited resource, > 2) Use some bits inside page->memcg_data - this I believe Johannes had > reasons against, and it isn't always the case that MEMCG support is > enabled. > 3) Use compound_dtor - but I think this would have problems for the > prior reasons. I don't think Michal is suggesting to use PageCompound() or PageHead(). He is suggesting to add a more general page flag (PageHasDestructor) and corresponding page->dtor, so other potential users can use it too. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shakeel Butt Subject: Re: [mm, net-next v2] mm: net: memcg accounting for TCP rx zerocopy Date: Wed, 24 Mar 2021 13:53:34 -0700 Message-ID: References: <20210316041645.144249-1-arjunroy.kdev@gmail.com> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9IAjIh7gf44rR4epDI0vFAr+lh0+8IoyQ48EbdutPC4=; b=X+i+XcHts81gTX2YQSoV3UFieH2kepMG7Wygr2n+8tt5NHgaQ+mstmTUI3J5ISoI7U SUl3tiJvsbwRVOLXnmugaf7VxQxxdhAWYfBnAG00WyqH9bu/e4yNA5k3oEymKHyTzkLh TTT+a8G71fKF6cc1A4Vkr/zD4HBlTVppNUf5AaI1iqxq87iXB7fbncGKeplOfgTH82ws hdeCH/aQeXDSvTbv1qPdy2Lh7zjA6lcDs1+f+DHw1q7iu3l5yphxkZpMqeDxCa8CTxyZ 5VTmYNZSiJct4WOQqCy68/ZoK5rI3L/qFEYbBPrXDsH1QW2R7QyRDf07jo8vRTObrEez ApCg== In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Arjun Roy Cc: Michal Hocko , Johannes Weiner , Arjun Roy , Andrew Morton , David Miller , netdev , Linux Kernel Mailing List , Cgroups , Linux MM , Eric Dumazet , Soheil Hassas Yeganeh , Jakub Kicinski , Yang Shi , Roman Gushchin On Wed, Mar 24, 2021 at 1:39 PM Arjun Roy wrote: > > On Wed, Mar 24, 2021 at 2:12 AM Michal Hocko wrote: > > > > On Tue 23-03-21 11:47:54, Arjun Roy wrote: > > > On Tue, Mar 23, 2021 at 7:34 AM Michal Hocko wrote: > > > > > > > > On Wed 17-03-21 18:12:55, Johannes Weiner wrote: > > > > [...] > > > > > Here is an idea of how it could work: > > > > > > > > > > struct page already has > > > > > > > > > > struct { /* page_pool used by netstack */ > > > > > /** > > > > > * @dma_addr: might require a 64-bit value even on > > > > > * 32-bit architectures. > > > > > */ > > > > > dma_addr_t dma_addr; > > > > > }; > > > > > > > > > > and as you can see from its union neighbors, there is quite a bit more > > > > > room to store private data necessary for the page pool. > > > > > > > > > > When a page's refcount hits zero and it's a networking page, we can > > > > > feed it back to the page pool instead of the page allocator. > > > > > > > > > > From a first look, we should be able to use the PG_owner_priv_1 page > > > > > flag for network pages (see how this flag is overloaded, we can add a > > > > > PG_network alias). With this, we can identify the page in __put_page() > > > > > and __release_page(). These functions are already aware of different > > > > > types of pages and do their respective cleanup handling. We can > > > > > similarly make network a first-class citizen and hand pages back to > > > > > the network allocator from in there. > > > > > > > > For compound pages we have a concept of destructors. Maybe we can extend > > > > that for order-0 pages as well. The struct page is heavily packed and > > > > compound_dtor shares the storage without other metadata > > > > int pages; /* 16 4 */ > > > > unsigned char compound_dtor; /* 16 1 */ > > > > atomic_t hpage_pinned_refcount; /* 16 4 */ > > > > pgtable_t pmd_huge_pte; /* 16 8 */ > > > > void * zone_device_data; /* 16 8 */ > > > > > > > > But none of those should really require to be valid when a page is freed > > > > unless I am missing something. It would really require to check their > > > > users whether they can leave the state behind. But if we can establish a > > > > contract that compound_dtor can be always valid when a page is freed > > > > this would be really a nice and useful abstraction because you wouldn't > > > > have to care about the specific type of page. > > > > > > > > But maybe I am just overlooking the real complexity there. > > > > -- > > > > > > For now probably the easiest way is to have network pages be first > > > class with a specific flag as previously discussed and have concrete > > > handling for it, rather than trying to establish the contract across > > > page types. > > > > If you are going to claim a page flag then it would be much better to > > have it more generic. Flags are really scarce and if all you care about > > is PageHasDestructor() and provide one via page->dtor then the similar > > mechanism can be reused by somebody else. Or does anything prevent that? > > The way I see it - the fundamental want here is, for some arbitrary > page that we are dropping a reference on, to be able to tell that the > provenance of the page is some network driver's page pool. If we added > an enum target to compound_dtor, if we examine that offset in the page > and look at that value, what guarantee do we have that the page isn't > instead some other kind of page, and the byte value there was just > coincidentally the one we were looking for (but it wasn't a network > driver pool page)? > > Existing users of compound_dtor seem to check first that a > PageCompound() or PageHead() return true - the specific scenario here, > of receiving network packets, those pages will tend to not be compound > (and more specifically, compound pages are explicitly disallowed for > TCP receive zerocopy). > > Given that's the case, the options seem to be: > 1) Use a page flag - with the downside that they are a severely > limited resource, > 2) Use some bits inside page->memcg_data - this I believe Johannes had > reasons against, and it isn't always the case that MEMCG support is > enabled. > 3) Use compound_dtor - but I think this would have problems for the > prior reasons. I don't think Michal is suggesting to use PageCompound() or PageHead(). He is suggesting to add a more general page flag (PageHasDestructor) and corresponding page->dtor, so other potential users can use it too.