[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/7] xen/gnttab: Rework resource acquisition
On 15.01.2021 17:57, Andrew Cooper wrote: > On 15/01/2021 11:56, Jan Beulich wrote: >>> + /* Grow table if necessary. */ >>> + grant_write_lock(gt); >>> + rc = -EINVAL; >>> + switch ( id ) >>> + { >>> + case XENMEM_resource_grant_table_id_shared: >>> + vaddrs = gt->shared_raw; >>> + rc = gnttab_get_shared_frame_mfn(d, tot_frames - 1, &tmp); >> ... this will degenerate (and still cause an error) when frame >> is also zero, and will cause undue growing of the table when >> frame is non-zero yet not overly large. > > Urgh, yes - that is why I had the check. > > In which case I retract my change between v2 and v3 here. > >> As indicated before, I'm of the clear opinion that here - like >> elsewhere - a number of zero frames requested means that no >> action be taken at all, and success be returned. > > The general world we work in (POSIX) agrees with my opinion over yours > when it comes to this matter. I assume you are referring to mmap()? I ask because I think there are numerous counter examples (some even in the C standard): malloc() & friends allow for either behavior. memcpy() / memmove() happily do nothing when passed a zero size. read() / write() are at least allowed to read/write nothing (and return success) when told so. Otoh I notice that a zero vector count passed to readv() / writev() is indeed an error, yet nothing is said at all about individual vector elements specifying zero size. Plus of course I don't think POSIX is the main reference point here, when the rest of the hypercalls allowing for some form of batching permit empty batches. > I spent a lot of time and effort getting this logic correct in v2, and I > do not have any further time to waste adding complexity to support a > non-existent corner case, nor is it reasonable to further delay all the > work which is depending on this series. This entire mess is already too > damn complicated, without taking extra complexity. > > Entertaining the idea of supporting 0 length requests is really not as > simple as you seem to think it is, and is a large part of why I'm > stubbornly refusing to do so. I'd be really happy to be educated of the complications; sadly so far you've only claimed ones would exist without actually going into sufficient detail. In particular I don't view placing if ( size == 0 ) return 0; suitably early coming anywhere near "complexity". Even more so that as per your reply you mean to undo removal of a respective check, just that in your version it'll return an error instead of success. > I am going to commit this patch (with some of the other minor adjustments). I'm not concerned enough of the introduced inconsistency to outright veto you doing so, but I still don't think this is an appropriate step to take under the present conditions. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |