Messages in this thread | | | Subject | Re: [PATCH 5/9] drm/xen-front: Implement handling of shared display buffers | From | Oleksandr Andrushchenko <> | Date | Fri, 23 Feb 2018 16:45:23 +0200 |
| |
On 02/23/2018 04:36 PM, Boris Ostrovsky wrote: > On 02/23/2018 02:53 AM, Oleksandr Andrushchenko wrote: >> On 02/23/2018 02:25 AM, Boris Ostrovsky wrote: >>> On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote: >>>> static int __init xen_drv_init(void) >>>> { >>>> + /* At the moment we only support case with XEN_PAGE_SIZE == >>>> PAGE_SIZE */ >>>> + BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE); >>> Why BUILD_BUG_ON? This should simply not load if page sizes are >>> different. >>> >>> >> This is a compile time check, so if kernel/Xen is configured >> to use page size combination which is not supported by the >> driver it will fail during compilation. This seems correct to me, >> because you shouldn't even try to load the driver which >> cannot handle different page sizes to not make any harm. > > This will prevent whole kernel from building. So, for example, > randconfig builds will fail. > makes a lot of sense, thank you will rework so I reject to load if the requirement is not met >>> >>> >>>> + ret = gnttab_map_refs(map_ops, NULL, buf->pages, buf->num_pages); >>>> + BUG_ON(ret); >>> We should try not to BUG*(). There are a few in this patch (and possibly >>> others) that I think can be avoided. >>> >> I will rework BUG_* for map/unmap code to handle errors, >> but will still leave >> /* either pages or sgt, not both */ >> BUG_ON(cfg->pages && cfg->sgt); >> which is a real driver bug and must not happen > Why not return an error? > > In fact, AFAICS you only call it in patch 9 where both of these can be > tested, in which case something like -EINVAL would look reasonable. ok, will remove BUG_ON as well > -boris
| |