Re: [PATCH 6/6] x86/xen: open code alloc_vm_area in arch_gnttab_valloc

2020-09-22 Thread boris . ostrovsky

On 9/22/20 11:27 AM, Christoph Hellwig wrote:
> On Tue, Sep 22, 2020 at 11:24:20AM -0400, boris.ostrov...@oracle.com wrote:
>> On 9/22/20 10:58 AM, Christoph Hellwig wrote:
>>> On Mon, Sep 21, 2020 at 04:44:10PM -0400, boris.ostrov...@oracle.com wrote:
 This will end up incrementing area->ptes pointer. So perhaps something like


 pte_t **ptes = area->ptes;

 if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
     PAGE_SIZE * nr_frames, gnttab_apply, )) {

    ...
>>> Yeah.  What do you think of this version? 
>>
>> Oh yes, this is way better. This now can actually be read without trying to 
>> mentally unwind triple pointers. (You probably want to initialize idx to 
>> zero before calling apply_to_page_range(), I am not sure it's guaranteed to 
>> be zero).
> Both instances are static variables, thus in .bss and initialized.
> So unless you insist I don't think we need a manual one.


Yes, you are right. (I thought perhaps this code could be called more than once 
but no, it can't).


-boris

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 6/6] x86/xen: open code alloc_vm_area in arch_gnttab_valloc

2020-09-22 Thread boris . ostrovsky

On 9/22/20 10:58 AM, Christoph Hellwig wrote:
> On Mon, Sep 21, 2020 at 04:44:10PM -0400, boris.ostrov...@oracle.com wrote:
>> This will end up incrementing area->ptes pointer. So perhaps something like
>>
>>
>> pte_t **ptes = area->ptes;
>>
>> if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
>>     PAGE_SIZE * nr_frames, gnttab_apply, )) {
>>
>>    ...
> Yeah.  What do you think of this version? 


Oh yes, this is way better. This now can actually be read without trying to 
mentally unwind triple pointers. (You probably want to initialize idx to zero 
before calling apply_to_page_range(), I am not sure it's guaranteed to be zero).


>  I think it is a little
> cleaner and matches what xenbus does.  At this point it probably should
> be split into a Xen and a alloc_vm_area removal patch, though.


Right.


-boris

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 6/6] x86/xen: open code alloc_vm_area in arch_gnttab_valloc

2020-09-21 Thread boris . ostrovsky

On 9/18/20 12:37 PM, Christoph Hellwig wrote:
>  
> +static int gnttab_apply(pte_t *pte, unsigned long addr, void *data)
> +{
> + pte_t ***p = data;
> +
> + **p = pte;
> + (*p)++;
> + return 0;
> +}
> +
>  static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned 
> nr_frames)
>  {
>   area->ptes = kmalloc_array(nr_frames, sizeof(*area->ptes), GFP_KERNEL);
>   if (area->ptes == NULL)
>   return -ENOMEM;
> -
> - area->area = alloc_vm_area(PAGE_SIZE * nr_frames, area->ptes);
> - if (area->area == NULL) {
> - kfree(area->ptes);
> - return -ENOMEM;
> - }
> -
> + area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_IOREMAP);
> + if (!area->area)
> + goto out_free_ptes;
> + if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
> + PAGE_SIZE * nr_frames, gnttab_apply, >ptes))


This will end up incrementing area->ptes pointer. So perhaps something like


pte_t **ptes = area->ptes;

if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
    PAGE_SIZE * nr_frames, gnttab_apply, )) {

   ...

}


-boris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel