From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753767AbeE3RtR (ORCPT ); Wed, 30 May 2018 13:49:17 -0400 Received: from mail-lf0-f68.google.com ([209.85.215.68]:38270 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753651AbeE3RtP (ORCPT ); Wed, 30 May 2018 13:49:15 -0400 X-Google-Smtp-Source: ADUXVKJKFTmKAp7fgIsUUIRnzBa1n96R1MW9Xj4x0zI1G74te0nKMuii7mlpYAJKDJCgW8q36zjpgg== Subject: Re: [PATCH 3/8] xen/grant-table: Allow allocating buffers suitable for DMA To: Boris Ostrovsky , xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, jgross@suse.com, konrad.wilk@oracle.com Cc: daniel.vetter@intel.com, dongwon.kim@intel.com, matthew.d.roper@intel.com, Oleksandr Andrushchenko References: <20180525153331.31188-1-andr2000@gmail.com> <20180525153331.31188-4-andr2000@gmail.com> <94de6bd7-405c-c43f-0468-be71efff7552@oracle.com> From: Oleksandr Andrushchenko Message-ID: <5e6e0f5d-a417-676a-1aad-c51eb09e6dee@gmail.com> Date: Wed, 30 May 2018 20:49:11 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/30/2018 06:20 PM, Boris Ostrovsky wrote: > On 05/30/2018 02:34 AM, Oleksandr Andrushchenko wrote: >> On 05/29/2018 10:10 PM, Boris Ostrovsky wrote: >>> On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote: >>> +/** >>> + * gnttab_dma_free_pages - free DMAable pages >>> + * @args: arguments to the function >>> + */ >>> +int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args) >>> +{ >>> +    xen_pfn_t *frames; >>> +    size_t size; >>> +    int i, ret; >>> + >>> +    gnttab_pages_clear_private(args->nr_pages, args->pages); >>> + >>> +    frames = kcalloc(args->nr_pages, sizeof(*frames), GFP_KERNEL); >>> >>> Any way you can do it without allocating memory? One possibility is to >>> keep allocated frames from gnttab_dma_alloc_pages(). (Not sure I like >>> that either but it's the only thing I can think of). >> Yes, I was also thinking about storing the allocated frames array from >> gnttab_dma_alloc_pages(), but that seemed not to be clear enough as >> the caller of the gnttab_dma_alloc_pages will need to store those frames >> in some context, so we can pass them on free. But the caller doesn't >> really >> need the frames which might confuse, so I decided to make those >> allocations >> on the fly. >> But I can still rework that to store the frames if you insist: please >> let me know. > > I would prefer not to allocate anything in the release path. Yes, I > realize that dragging frames array around is not necessary but IMO it's > better than potentially failing an allocation during a teardown. A > comment in the struct definition could explain the reason for having > this field. Then I would suggest we have it this way: current API requires that struct page **pages are allocated from outside. So, let's allocate the frames from outside as well. This way the caller is responsible for both pages and frames arrays and API looks consistent. > >>> >>>> +    if (!frames) >>>> +        return -ENOMEM; >>>> + >>>> +    for (i = 0; i < args->nr_pages; i++) >>>> +        frames[i] = page_to_xen_pfn(args->pages[i]); >>> Not xen_page_to_gfn()? >> Well, according to [1] it should be : >>     /* XENMEM_populate_physmap requires a PFN based on Xen >>      * granularity. >>      */ >>     frame_list[i] = page_to_xen_pfn(page); > > Ah, yes. I was looking at decrease_reservation and automatically assumed > the same parameter type. Good, then this one is resolved > > -boris > > Thank you, Oleksandr