Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <24748b59-e2ed-1d7d-801e-ab9dc12b0983@oracle.com>
Date:   Wed, 11 May 2022 17:09:33 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.9.0
Subject: Re: [PATCH V2 2/7] xen/grants: support allocating consecutive grants
Content-Language: en-US
To:     Oleksandr <olekstysh@gmail.com>,
        Stefano Stabellini <sstabellini@kernel.org>
Cc:     xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Juergen Gross <jgross@suse.com>, Julien Grall <julien@xen.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Christoph Hellwig <hch@infradead.org>
References: <1651947548-4055-1-git-send-email-olekstysh@gmail.com>
 <1651947548-4055-3-git-send-email-olekstysh@gmail.com>
 <8409e636-94e3-e231-f181-c76a849534a1@gmail.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <8409e636-94e3-e231-f181-c76a849534a1@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aWlBQnN3R2xYRnQzWjJuNWxhS2RTNGJ3NjJFYktDSEJka2hndWNHcnlqZWJF?=
 =?utf-8?B?S2hSOXBkQ1JNMWxoWVg3VFV3a3Mzd0g1cVJQUWZmUi90ZDRIYXhzWlBubUxp?=
 =?utf-8?B?RlBPeHNtWldYYzlUNkRtV0xyY2NnVWNXN1BKUTJESXFKbmFrNHdDZkZHYTVq?=
 =?utf-8?B?bzU1ZDRSRlp3Vkh5cFdwemFvanFEaVVyaGhOR0c5akdRWitueS9SMEJSVS96?=
 =?utf-8?B?TzJWZldOdTd1QnZQM3lCQ1RSaWZNWTFZTTkwbWNjOWFjRXk3VnZ3d1l3dFNZ?=
 =?utf-8?B?bmY2ckJLeGtPNW5LdEVBNlhybEpvQnJnSngzV3FqUnVUNWp5WVVkbDg2RGhs?=
 =?utf-8?B?ZENoNG0reW9oeGxndXVVQkNtN0ZOT00rWVVuS1c5TUJlK0JxcGdDYnJYUTlG?=
 =?utf-8?B?ZlNpOFIzSEl1ZjJuU3VZakE3b0pZVVhWNW13SVU0K3E5ZzNZanRJVVFqSEp1?=
 =?utf-8?B?eEhGUm1jbkRBWkJRLzFla0hnQU5VZ2lLaGZRNmdjS2x0K0h1T0MrcGZBb3dr?=
 =?utf-8?B?cTJ1ajJKOXhaam1qelE0NEU5VWtBYksydFJuS1NUVmk3UFRGb2xldEVhdTJL?=
 =?utf-8?B?d1E2VGN5SWlBeWFjWno4WG1VdXpnWEdqMHRqT2crMUdGMlpoRlZia2lsemF4?=
 =?utf-8?B?MUk0dEdEbUIza3RCZXg4ZzlDeW91RjdhQmxaMUUwU1JrSllrTjN4SHFDSWlt?=
 =?utf-8?B?V3l6TDc4dittTlpSZzkvZzMzMWwyN2lMRXNFZ2R3Vk5CUW1hV2o5T1FERUFr?=
 =?utf-8?B?VHFwQ2xST3AyMThLakMxdXRWZ0M1bW9yeUw2S2hDTUZNU3l2UUZzRU9VUGV6?=
 =?utf-8?B?TE9sdTN4RlZlbmVZdmVVaGdHZ1JYVFRWa2w5UHpmV1BLWGpXbUZoWE9FUGUz?=
 =?utf-8?B?cEs0elptVVJyemxDdUlJNFE2dy9KS2hoWncwemVqZ1FpRVVOckRmUCsyOTdo?=
 =?utf-8?B?MVJTdWV0Ui9zV3M5MENvZGY3RzE3NG9nanJoUUtVeGRXVVJGSHEvcGJrdzZp?=
 =?utf-8?B?QXFTVlIwV2JDYkJUQjM0VFlPUzlFcGN6YVhYTzE5aUcvVTZDMTJRU1QyT01j?=
 =?utf-8?B?dTlQOGRYZ1NNOVB0TXBKU000SVlyZjBxWXRMK001ZWk2WjhNbXJwdWp4WGxw?=
 =?utf-8?B?K1FNeXo1cmpURjRBcnhrY1MrcTY1eGtQdmkyWERGd04xNFdKWUl5NzFrOHAx?=
 =?utf-8?B?SlZWTFVndDRsaG5BWnAycDVIWWwxTjJXUXNyRmg0eHVNVjFScVE2dW9CeURB?=
 =?utf-8?B?MHVEZHd5eXEwSzJjQ05nUUNGVCtRREJjREJSck8xN1RtUlVjbi9VUmtvOXJz?=
 =?utf-8?B?dytIZjgxQkQ0NVFVK2IzcCtZaVpBS1ZXMjlLQk5LNWtpdVNPZi9YYmZpd25U?=
 =?utf-8?B?QysrYlE5amc2SkNvc3NoUnFxd2tSNjY3Z2N2LzhPSHN3eDlIVDRSenlEQlUz?=
 =?utf-8?B?by9wbWpxaTBKN3JoSlNwNkpLODZRemVWcHduR3lDd1hxU1BPNm9LbFN5WGh2?=
 =?utf-8?B?b2JlT3FHb1dLWTBsVkg0eWlKQTliRUQ3UGJwN3RoNm9yMmZBdjFERStkT2RH?=
 =?utf-8?B?SEwvaG9kLy9qK3RZODJIUWNZSURaNCtJSVBETUU4VFl2dmtKclh2MmNxVmlH?=
 =?utf-8?B?S1VWQVZKZkt5VkJlT2tPR3FHUzh0SVFEejNTMlJYQWdBdlhPNFNHcTVENEg1?=
 =?utf-8?B?SmlrdzRpamJ3UVQyY3pPcGsvU0VBbEpYcVN2MVhWUzdSTXlxa1NnSmZoMGhz?=
 =?utf-8?B?ck1NV0p2Q1pzVWJOY1Z0MlU0SXlWUlhOMEVvSlRYMlRxaXh1US96ZklrN2la?=
 =?utf-8?B?Z0I2Q0NjaS9oSEJZemR1dmZOTGlLMW1ZN1NCU2lGMEJBdDhtMWxhQWY3R1Yv?=
 =?utf-8?B?THVjdHRMRmx2Nm1GMnpDK3BVSUoxRW5LYkZDeVVIZGIzN0h6YkNwWWRsWTVa?=
 =?utf-8?B?bkphTkt6bDhNK3o3aWpUQWZ5dWJWUlo4VHN5SkdCMS8xRUdsU3BhbEJwQnA0?=
 =?utf-8?B?M3NFSVI2NVFDTlNBRk80SVNMTktYcytldVBOMmJqYjkrYUd1MzZ5Tll6L3A4?=
 =?utf-8?B?VU5sZjRKSWJXZUtOMWw0bUVrS0lQZUlCdnp4bEhaeHdwTlBDMDNweHpaVFNt?=
 =?utf-8?B?WnJBTDNCVzU3Z25FU3U4YVdrU2V0dXNKVUN5SDVLS2VDSXBORDZVSzlBTVFE?=
 =?utf-8?B?OTJOZzhFdndCZjdNQ1MyMnV3Nm42aWdSTEFmbWRIc1B3WWhFSVhJK0RiSjBs?=
 =?utf-8?B?M1phRitzdzVUQ1pUTDFQUEl0V01HdG13eStSOUMrQzJKU3BDYkRIeGwyU3NQ?=
 =?utf-8?B?TU5Jek8xWWZFYlNFWlZPTFltWmdFYnN3aHhDQUJoYTk0L2RxTURFVm0yT0p4?=
 =?utf-8?Q?W/rjUGlTk89hs0QM=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 00d2aae7-7c63-4434-e8ea-08da33928f25
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 May 2022 21:09:38.8726
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SEif40vTMr5TYc2Qku+3LVVncd4ooM9Hco2gBDpcL2sshp1lcCWlldVBfsmFndQKCk1eij+UXJmKU8/yCUiGijFLmLDlszaBEDjsfC4fXkY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB3872
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-11_07:2022-05-11,2022-05-11 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=804 malwarescore=0
 suspectscore=0 phishscore=0 spamscore=0 bulkscore=0 mlxscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205110091
X-Proofpoint-ORIG-GUID: e3AavS7o_eFKu7AeDx8tI1DYYUovFplk
X-Proofpoint-GUID: e3AavS7o_eFKu7AeDx8tI1DYYUovFplk
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 5/11/22 2:00 PM, Oleksandr wrote:
>
> On 07.05.22 21:19, Oleksandr Tyshchenko wrote:
>
> Hello Boris, Stefano
>
>
>> From: Juergen Gross <jgross@suse.com>
>>
>> For support of virtio via grant mappings in rare cases larger mappings
>> using consecutive grants are needed. Support those by adding a bitmap
>> of free grants.
>>
>> As consecutive grants will be needed only in very rare cases (e.g. when
>> configuring a virtio device with a multi-page ring), optimize for the
>> normal case of non-consecutive allocations.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> Changes RFC -> V1:
>>     - no changes
>>     Changes V1 -> V2:
>>     - no changes
>
>
> May I please ask for the review here?


I had a quick look but I am stuck on get_free_seq(), I need to stare at it some more. Unless someone else reviews this, I will try to get to this in the next couple of days.


One thing I did notice is


>
>> @@ -1452,6 +1624,13 @@ int gnttab_init(void)
>>           }
>>       }
>>   +    i = gnttab_interface->grefs_per_grant_frame * max_nr_grant_frames;
>> +    gnttab_free_bitmap = bitmap_zalloc(i, GFP_KERNEL);
>> +    if (!gnttab_free_bitmap) {
>> +        ret = -ENOMEM;
>> +        goto ini_nomem;
>> +    }


This overwrites 'i' and will break error handling at ini_nomem.


-boris