netdev - Re: [PATCH 3/3] net/xdp: convert put_page() to put_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Jul 2019 17:25:34 -0700
From:   Ira Weiny <ira.weiny@...el.com>
To:     john.hubbard@...il.com
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Björn Töpel <bjorn.topel@...el.com>,
        Boaz Harrosh <boaz@...xistor.com>,
        Christoph Hellwig <hch@....de>,
        Daniel Vetter <daniel@...ll.ch>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Chinner <david@...morbit.com>,
        David Airlie <airlied@...ux.ie>,
        "David S . Miller" <davem@...emloft.net>,
        Ilya Dryomov <idryomov@...il.com>, Jan Kara <jack@...e.cz>,
        Jason Gunthorpe <jgg@...pe.ca>, Jens Axboe <axboe@...nel.dk>,
        Jérôme Glisse <jglisse@...hat.com>,
        Johannes Thumshirn <jthumshirn@...e.de>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Matthew Wilcox <willy@...radead.org>,
        Miklos Szeredi <miklos@...redi.hu>,
        Ming Lei <ming.lei@...hat.com>, Sage Weil <sage@...hat.com>,
        Santosh Shilimkar <santosh.shilimkar@...cle.com>,
        Yan Zheng <zyan@...hat.com>, netdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-mm@...ck.org,
        linux-rdma@...r.kernel.org, bpf@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        John Hubbard <jhubbard@...dia.com>
Subject: Re: [PATCH 3/3] net/xdp: convert put_page() to put_user_page*()

On Mon, Jul 22, 2019 at 03:34:15PM -0700, john.hubbard@...il.com wrote:
> From: John Hubbard <jhubbard@...dia.com>
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> Cc: Björn Töpel <bjorn.topel@...el.com>
> Cc: Magnus Karlsson <magnus.karlsson@...el.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: netdev@...r.kernel.org
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---
>  net/xdp/xdp_umem.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 83de74ca729a..0325a17915de 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -166,14 +166,7 @@ void xdp_umem_clear_dev(struct xdp_umem *umem)
>  
>  static void xdp_umem_unpin_pages(struct xdp_umem *umem)
>  {
> -	unsigned int i;
> -
> -	for (i = 0; i < umem->npgs; i++) {
> -		struct page *page = umem->pgs[i];
> -
> -		set_page_dirty_lock(page);
> -		put_page(page);
> -	}
> +	put_user_pages_dirty_lock(umem->pgs, umem->npgs);

What is the difference between this and

__put_user_pages(umem->pgs, umem->npgs, PUP_FLAGS_DIRTY_LOCK);

?

I'm a bit concerned with adding another form of the same interface.  We should
either have 1 call with flags (enum in this case) or multiple calls.  Given the
previous discussion lets move in the direction of having the enum but don't
introduce another caller of the "old" interface.

So I think on this patch NAK from me.

I also don't like having a __* call in the exported interface but there is a
__get_user_pages_fast() call so I guess there is precedent.  :-/

Ira

>  
>  	kfree(umem->pgs);
>  	umem->pgs = NULL;
> -- 
> 2.22.0
>