Hi Sakari, On Thursday, 13 December 2018 15:02:03 EET Sakari Ailus wrote: > On Thu, Dec 13, 2018 at 02:59:50PM +0200, Laurent Pinchart wrote: > > On Thursday, 13 December 2018 12:40:06 EET Sakari Ailus wrote: > > > The size argument of the alloc memop, which allocates buffer memory, is > > > page aligned. Document it as such, as code elsewhere has not taken this > > > into account. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > > --- > > > > > > include/media/videobuf2-core.h | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/media/videobuf2-core.h > > > b/include/media/videobuf2-core.h index e86981d615ae4..68b9fe660e4f1 > > > 100644 > > > --- a/include/media/videobuf2-core.h > > > +++ b/include/media/videobuf2-core.h > > > @@ -54,7 +54,8 @@ struct vb2_threadio_data; > > > * will then be passed as @buf_priv argument to other ops in this > > > * structure. Additional gfp_flags to use when allocating the > > > * are also passed to this operation. These flags are from the > > > - * gfp_flags field of vb2_queue. > > > + * gfp_flags field of vb2_queue. The size argument to this function > > > + * shall be *page aligned*. > > > * @put: inform the allocator that the buffer will no longer be used; > > > * usually will result in the allocator freeing the buffer (if > > > * no other users of this buffer are present); the @buf_priv > > > > I wonder if a WARN_ON() to ensure this would make sense. In any case, > > There's a single place where the alloc() op is called. I thought it'd be > silly to put a check after the line of code that performs the alignment. I was thinking about WARN_ON in the memory allocators, but if it's called from a single place, I agree it may not be needed. A comment in the dma-sg allocator would be useful though. > Perhaps a comment right there? > > I'm open to other ideas that don't seem silly. :-) Stop calling my ideas silly :-) > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> -- Regards, Laurent Pinchart
