Re: [PATCH v2 7/9] xen/gntdev: Implement dma-buf export functionality

[Oleksandr Andrushchenko]

On 06/08/2018 01:30 AM, Boris Ostrovsky wrote:
> On 06/07/2018 04:44 AM, Oleksandr Andrushchenko wrote:
> > On 06/07/2018 12:48 AM, Boris Ostrovsky wrote:
> > > On 06/06/2018 08:10 AM, Oleksandr Andrushchenko wrote:
> > > > On 06/05/2018 01:07 AM, Boris Ostrovsky wrote:
> > > > > On 06/01/2018 07:41 AM, Oleksandr Andrushchenko wrote:
> > > > > +
> > > > > +static struct sg_table *
> > > > > +dmabuf_exp_ops_map_dma_buf(struct dma_buf_attachment *attach,
> > > > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ enum dma_data_direction dir)
> > > > > +{
> > > > > +ÂÂÂ struct gntdev_dmabuf_attachment *gntdev_dmabuf_attach =
> > > > > attach->priv;
> > > > > +ÂÂÂ struct gntdev_dmabuf *gntdev_dmabuf = attach->dmabuf->priv;
> > > > > +ÂÂÂ struct sg_table *sgt;
> > > > > +
> > > > > +ÂÂÂ pr_debug("Mapping %d pages for dev %p\n",
> > > > > gntdev_dmabuf->nr_pages,
> > > > > +ÂÂÂÂÂÂÂÂ attach->dev);
> > > > > +
> > > > > +ÂÂÂ if (WARN_ON(dir == DMA_NONE || !gntdev_dmabuf_attach))
> > > > >
> > > > > WARN_ON_ONCE. Here and elsewhere.
> > > > Why? The UAPI may be used by different applications, thus we might
> > > > lose warnings for some of them. Having WARN_ON will show problems
> > > > for multiple users, not for the first one.
> > > > Does this make sense to still use WARN_ON?
> > > Just as with pr_err call somewhere else the concern here is that
> > > userland (which I think is where this is eventually called from?) may
> > > intentionally trigger the error, flooding the log.
> > >
> > > And even this is not directly called from userland there is still a
> > > possibility of triggering this error multiple times.
> > Ok, will use WARN_ON_ONCE
>
> In fact, is there a reason to use WARN at all? Does this condition
> indicate some sort of internal inconsistency/error?
Well, the corresponding errors are anyways handled, so I will remove WARN
> -boris