Linaro-mm-sig October 2016

linaro-mm-sig@lists.linaro.org

10 participants
7 discussions

[PATCH] allow buffers allocation with DMABUF memory type

by Benjamin Gaignard

Until now the only way to make the driver allocate buffers and share them using dma_buf was to use V4L2_MEMORY_MMAP memory type. Use of MMAP memory type is a problem because vb2 never call dma_buf_map_attachment() to attach itself while queuing the buffer so dma_buf importer will not know that another device use this buffer. This patch allow to allocate buffer even for DMABUF memory type and correctly manage dma_buf buffer attachment. vb2_mem_ops attach_dmabuf() prototype has been changed to be able to distinguish if the attachment is done on a already existing buffer or on imported one. Signed-off-by: Benjamin Gaignard <benjamin.gaignard(a)linaro.org> --- drivers/media/v4l2-core/videobuf2-core.c | 76 ++++++++++++++++++++++++-- drivers/media/v4l2-core/videobuf2-dma-contig.c | 10 ++-- drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 ++- drivers/media/v4l2-core/videobuf2-vmalloc.c | 8 ++- include/media/videobuf2-core.h | 5 +- 5 files changed, 91 insertions(+), 16 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 1329dcc..c5968aa 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -337,6 +337,67 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n) } /** + * __setup_dmabufs() - setup dmabuf fd for every plane in + * every buffer on the queue + */ +static void __setup_dmabufs(struct vb2_queue *q, unsigned int n) +{ + unsigned int buffer, plane; + struct vb2_buffer *vb; + struct vb2_plane *vb_plane; + struct dma_buf *dbuf; + void *mem_priv; + int fd; + int write = !V4L2_TYPE_IS_OUTPUT(q->type); + int flags = write ? O_WRONLY : O_RDONLY; + + for (buffer = q->num_buffers; buffer < q->num_buffers + n; ++buffer) { + vb = q->bufs[buffer]; + if (!vb) + continue; + + for (plane = 0; plane < vb->num_planes; ++plane) { + vb_plane = &vb->planes[plane]; + + dbuf = call_ptr_memop(vb, get_dmabuf, + vb_plane->mem_priv, + flags & O_ACCMODE); + if (IS_ERR_OR_NULL(dbuf)) { + dprintk(1, "Failed to export buffer %d, " \ + "plane %d\n", buffer, plane); + continue; + } + + fd = dma_buf_fd(dbuf, flags & ~O_ACCMODE); + if (fd < 0) { + dprintk(3, "buffer %d, plane %d failed " \ + "to export (%d)\n", buffer, plane, fd); + dma_buf_put(dbuf); + continue; + } + + /* Acquire each plane's memory */ + mem_priv = call_ptr_memop(vb, attach_dmabuf, + q->alloc_ctx[plane], dbuf, + vb->v4l2_planes[plane].length, + write, vb_plane->mem_priv); + if (IS_ERR(mem_priv)) { + dprintk(1, "Buffer %d plane %d failed "\ + "to attach dmabuf\n", buffer, plane); + dma_buf_put(dbuf); + continue; + } + + vb_plane->dbuf = dbuf; + vb->v4l2_planes[plane].m.fd = fd; + + dprintk(3, "Buffer %d, plane %d fd %x\n", + buffer, plane, fd); + } + } +} + +/** * __vb2_queue_alloc() - allocate videobuf buffer structures and (for MMAP type) * video buffer memory for all buffers/planes on the queue and initializes the * queue @@ -369,8 +430,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, vb->v4l2_buf.type = q->type; vb->v4l2_buf.memory = memory; - /* Allocate video buffer memory for the MMAP type */ - if (memory == V4L2_MEMORY_MMAP) { + /* Allocate video buffer memory for the MMAP and DMABUF types */ + if (memory == V4L2_MEMORY_MMAP || + memory == V4L2_MEMORY_DMABUF) { ret = __vb2_buf_mem_alloc(vb); if (ret) { dprintk(1, "failed allocating memory for " @@ -400,6 +462,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory, if (memory == V4L2_MEMORY_MMAP) __setup_offsets(q, buffer); + if (memory == V4L2_MEMORY_DMABUF) + __setup_dmabufs(q, buffer); + dprintk(1, "allocated %d buffers, %d plane(s) each\n", buffer, num_planes); @@ -859,7 +924,7 @@ static int __verify_memory_type(struct vb2_queue *q, * to be used during streaming, * 4) allocates internal buffer structures (struct vb2_buffer), according to * the agreed parameters, - * 5) for MMAP memory type, allocates actual video memory, using the + * 5) for MMAP and DMABUF memory types, allocates actual video memory, using the * memory handling/allocation routines provided during queue initialization * * If req->count is 0, all the memory will be freed instead. @@ -885,7 +950,8 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) * are not in use and can be freed. */ mutex_lock(&q->mmap_lock); - if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) { + if ((q->memory == V4L2_MEMORY_MMAP || + q->memory == V4L2_MEMORY_DMABUF) && __buffers_in_use(q)) { mutex_unlock(&q->mmap_lock); dprintk(1, "memory in use, cannot free\n"); return -EBUSY; @@ -1540,7 +1606,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) /* Acquire each plane's memory */ mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane], - dbuf, planes[plane].length, dma_dir); + dbuf, planes[plane].length, dma_dir, NULL); if (IS_ERR(mem_priv)) { dprintk(1, "failed to attach dmabuf\n"); ret = PTR_ERR(mem_priv); diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index b481d20..8dc945e 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -793,7 +793,7 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) struct vb2_dc_buf *buf = mem_priv; /* if vb2 works correctly you should never detach mapped buffer */ - if (WARN_ON(buf->dma_addr)) + if (WARN_ON(buf->dma_sgt)) vb2_dc_unmap_dmabuf(buf); /* detach this attachment */ @@ -802,16 +802,18 @@ static void vb2_dc_detach_dmabuf(void *mem_priv) } static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { struct vb2_dc_conf *conf = alloc_ctx; - struct vb2_dc_buf *buf; + struct vb2_dc_buf *buf = buf_priv; struct dma_buf_attachment *dba; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c index b1838ab..b9855fd 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c @@ -667,16 +667,18 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv) } static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { struct vb2_dma_sg_conf *conf = alloc_ctx; - struct vb2_dma_sg_buf *buf; + struct vb2_dma_sg_buf *buf = buf_priv; struct dma_buf_attachment *dba; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c index bcde885..373294c 100644 --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c @@ -416,14 +416,16 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv) } static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, - unsigned long size, enum dma_data_direction dma_dir) + unsigned long size, enum dma_data_direction dma_dir, void *buf_priv) { - struct vb2_vmalloc_buf *buf; + struct vb2_vmalloc_buf *buf = buf_priv; if (dbuf->size < size) return ERR_PTR(-EFAULT); - buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) + buf = kzalloc(sizeof(*buf), GFP_KERNEL); + if (!buf) return ERR_PTR(-ENOMEM); diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index a5790fd..374b26b 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -49,6 +49,8 @@ struct vb2_threadio_data; * used for DMABUF memory types; alloc_ctx is the alloc context * dbuf is the shared dma_buf; returns NULL on failure; * allocator private per-buffer structure on success; + * if private per-buffer structure is provided reuse it + * instead of allocating a new one. * this needs to be used for further accesses to the buffer. * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF * buffer is no longer used; the buf_priv argument is the @@ -98,7 +100,8 @@ struct vb2_mem_ops { void *(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf, unsigned long size, - enum dma_data_direction dma_dir); + enum dma_data_direction dma_dir, + void *buf_priv); void (*detach_dmabuf)(void *buf_priv); int (*map_dmabuf)(void *buf_priv); void (*unmap_dmabuf)(void *buf_priv); -- 1.9.1

7 years, 2 months

[PATCH] dma-buf: Wait on the reservation object when sync'ing before CPU access

by Chris Wilson

Rendering operations to the dma-buf are tracked implicitly via the reservation_object (dmabuf->resv). This is used to allow poll() to wait upon outstanding rendering (or just query the current status of rendering). The dma-buf sync ioctl allows userspace to prepare the dma-buf for CPU access, which should include waiting upon rendering. (Some drivers may need to do more work to ensure that the dma-buf mmap is coherent as well as complete.) Signed-off-by: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-kernel(a)vger.kernel.org --- I'm wondering whether it makes sense just to always do the wait first. It is one of the first operations every driver has to make. A driver that wants to implement it differently (e.g. they can special case native waits) will still require a wait on the reservation object to finish external rendering. -Chris --- drivers/dma-buf/dma-buf.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index ddaee60ae52a..123f14b8e882 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, } EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment); +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) +{ + bool write = (direction == DMA_BIDIRECTIONAL || + direction == DMA_TO_DEVICE); + struct reservation_object *resv = dma_buf->resv; + long ret; + + /* Wait on any implicit rendering fences */ + ret = reservation_object_wait_timeout_rcu(resv, write, true, + MAX_SCHEDULE_TIMEOUT); + if (ret < 0) + return ret; + + return 0; +} /** * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the @@ -607,6 +623,8 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf, if (dmabuf->ops->begin_cpu_access) ret = dmabuf->ops->begin_cpu_access(dmabuf, direction); + else + ret = __dma_buf_begin_cpu_access(dmabuf, direction); return ret; } -- 2.8.1

7 years, 3 months

[PATCH -next] dma-buf/sw_sync: fix non static symbol warning

by Wei Yongjun

From: Wei Yongjun <weiyongjun1(a)huawei.com> Fixes the following sparse warning: drivers/dma-buf/sw_sync.c:87:22: warning: symbol 'sync_timeline_create' was not declared. Should it be static? Signed-off-by: Wei Yongjun <weiyongjun1(a)huawei.com> --- drivers/dma-buf/sw_sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 82e0ca4..7aa4d7b 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -84,7 +84,7 @@ static inline struct sync_pt *dma_fence_to_sync_pt(struct dma_fence *fence) * Creates a new sync_timeline. Returns the sync_timeline object or NULL in * case of error. */ -struct sync_timeline *sync_timeline_create(const char *name) +static struct sync_timeline *sync_timeline_create(const char *name) { struct sync_timeline *obj;

7 years, 5 months

[RFC 0/6] Module for tracking/accounting shared memory buffers

by Ruchi Kandoi

This patchstack introduces a new "memtrack" module for tracking and accounting memory exported to userspace as shared buffers, like dma-buf fds or GEM handles. Any process holding a reference to these buffers will keep the kernel from reclaiming its backing pages. mm counters don't provide a complete picture of these allocations, since they only account for pages that are mapped into a process's address space. This problem is especially bad for systems like Android that use dma-buf fds to share graphics and multimedia buffers between processes: these allocations are often large, have complex sharing patterns, and are rarely mapped into every process that holds a reference to them. memtrack maintains a per-process list of shared buffer references, which is exported to userspace as /proc/[pid]/memtrack. Buffers can be optionally "tagged" with a short string: for example, Android userspace would use this tag to identify whether buffers were allocated on behalf of the camera stack, GL, etc. memtrack also exports the VMAs associated with these buffers so that pages already included in the process's mm counters aren't double-counted. Shared-buffer allocators can hook into memtrack by embedding struct memtrack_buffer in their buffer metadata, calling memtrack_buffer_{init,remove} at buffer allocation and free time, and memtrack_buffer_{install,uninstall} when a userspace process takes or drops a reference to the buffer. For fd-backed buffers like dma-bufs, hooks in fdtable.c and fork.c automatically notify memtrack when references are added or removed from a process's fd table. This patchstack adds memtrack hooks into dma-buf and ion. If there's upstream interest in memtrack, it can be extended to other memory allocators as well, such as GEM implementations. Greg Hackmann (1): drivers: staging: ion: add ION_IOC_TAG ioctl Ruchi Kandoi (5): fs: add installed and uninstalled file_operations drivers: misc: add memtrack dma-buf: add memtrack support memtrack: Adds the accounting to keep track of all mmaped/unmapped pages. memtrack: Add memtrack accounting for forked processes. drivers/android/binder.c | 4 +- drivers/dma-buf/dma-buf.c | 37 +++ drivers/misc/Kconfig | 16 + drivers/misc/Makefile | 1 + drivers/misc/memtrack.c | 516 ++++++++++++++++++++++++++++++++ drivers/staging/android/ion/ion-ioctl.c | 17 ++ drivers/staging/android/ion/ion.c | 60 +++- drivers/staging/android/ion/ion_priv.h | 2 + drivers/staging/android/uapi/ion.h | 25 ++ fs/file.c | 38 ++- fs/open.c | 2 +- fs/proc/base.c | 4 + include/linux/dma-buf.h | 5 + include/linux/fdtable.h | 4 +- include/linux/fs.h | 2 + include/linux/memtrack.h | 130 ++++++++ include/linux/mm.h | 3 + include/linux/sched.h | 3 + kernel/fork.c | 23 +- 19 files changed, 875 insertions(+), 17 deletions(-) create mode 100644 drivers/misc/memtrack.c create mode 100644 include/linux/memtrack.h -- 2.8.0.rc3.226.g39d4020

7 years, 6 months

[PATCH v10 0/3] Secure Memory Allocation Framework

by Benjamin Gaignard

version 10 changes: - rebased on kernel 4.8 tag - minor typo fix version 9 changes: - rebased on 4.8-rc5 - struct dma_attrs doesn't exist anymore so update CMA allocator to compile with new dma_*_attr functions - add example SMAF use case in cover letter version 8 changes: - rework of the structures used within ioctl by adding a version field and padding to be futur proof - rename fake secure moduel to test secure module - fix the various remarks done on the previous patcheset version 7 changes: - rebased on kernel 4.6-rc7 - simplify secure module API - add vma ops to be able to detect mmap/munmap calls - add ioctl to get number and allocator names - update libsmaf with adding tests https://git.linaro.org/people/benjamin.gaignard/libsmaf.git - add debug log in fake secure module version 6 changes: - rebased on kernel 4.5-rc4 - fix mmapping bug while requested allocation size isn't a a multiple of PAGE_SIZE (add a test for this in libsmaf) version 5 changes: - rebased on kernel 4.3-rc6 - rework locking schema and make handle status use an atomic_t - add a fake secure module to allow performing tests without trusted environment version 4 changes: - rebased on kernel 4.3-rc3 - fix missing EXPORT_SYMBOL for smaf_create_handle() version 3 changes: - Remove ioctl for allocator selection instead provide the name of the targeted allocator with allocation request. Selecting allocator from userland isn't the prefered way of working but is needed when the first user of the buffer is a software component. - Fix issues in case of error while creating smaf handle. - Fix module license. - Update libsmaf and tests to care of the SMAF API evolution https://git.linaro.org/people/benjamin.gaignard/libsmaf.git version 2 changes: - Add one ioctl to allow allocator selection from userspace. This is required for the uses case where the first user of the buffer is a software IP which can't perform dma_buf attachement. - Add name and ranking to allocator structure to be able to sort them. - Create a tiny library to test SMAF: https://git.linaro.org/people/benjamin.gaignard/libsmaf.git - Fix one issue when try to secure buffer without secure module registered SMAF aim to solve two problems: allocating memory that fit with hardware IPs constraints and secure those data from bus point of view. One example of SMAF usage is camera preview: on SoC you may use either an USB webcam or the built-in camera interface and the frames could be send directly to the dipslay Ip or handle by GPU. Most of USB interfaces and GPU have mmu but almost all built-in camera interace and display Ips don't have mmu so when selecting how allocate buffer you need to be aware of each devices constraints (contiguous memroy, stride, boundary, alignment ...). ION has solve this problem by let userland decide which allocator (heap) to use but this require to adapt userland for each platform and sometime for each use case. To be sure to select the best allocation method for devices SMAF implement deferred allocation mechanism: memory allocation is only done when the first device effectively required it. Allocator modules have to implement a match() to let SMAF know if they are compatibles with devices needs. This patch set provide an example of allocator module which use dma_{alloc/free/mmap}_attrs() and check if at least one device have coherent_dma_mask set to DMA_BIT_MASK(32) in match function. In the same camera preview use case, SMAF allow to protect the data from being read by unauthorized IPs (i.e. a malware to dump camera stream). Until now I have only see access rights protection at process/thread level (PKeys/MPK) or on file (SELinux) but nothing allow to drive data bus firewalls. SMAF propose an interface to control and implement those firewalls. Like IOMMU, firewalls IPs can help to protect memory from malicious/faulty devices that are attempting DMA attacks. Secure modules are responsibles of granting and revoking devices access rights on the memory. Secure module is also called to check if CPU map memory into kernel and user address spaces. An example of secure module implementation can be found here: http://git.linaro.org/people/benjamin.gaignard/optee-sdp.git This code isn't yet part of the patch set because it depends on generic TEE which is still under discussion (https://lwn.net/Articles/644646/) For allocation part of SMAF code I get inspirated by Sumit Semwal work about constraint aware allocator. Benjamin Gaignard (3): create SMAF module SMAF: add CMA allocator SMAF: add test secure module drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/smaf/Kconfig | 17 + drivers/smaf/Makefile | 3 + drivers/smaf/smaf-cma.c | 186 ++++++++++ drivers/smaf/smaf-core.c | 818 +++++++++++++++++++++++++++++++++++++++++ drivers/smaf/smaf-testsecure.c | 90 +++++ include/linux/smaf-allocator.h | 45 +++ include/linux/smaf-secure.h | 65 ++++ include/uapi/linux/smaf.h | 85 +++++ 10 files changed, 1312 insertions(+) create mode 100644 drivers/smaf/Kconfig create mode 100644 drivers/smaf/Makefile create mode 100644 drivers/smaf/smaf-cma.c create mode 100644 drivers/smaf/smaf-core.c create mode 100644 drivers/smaf/smaf-testsecure.c create mode 100644 include/linux/smaf-allocator.h create mode 100644 include/linux/smaf-secure.h create mode 100644 include/uapi/linux/smaf.h -- 1.9.1

7 years, 6 months

[PATCH 00/15] improve function-level documentation

by Julia Lawall

These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely. // <smpl> @initialize:ocaml@ @@ let tbl = ref [] let fnstart = ref [] let success = Hashtbl.create 101 let thefile = ref "" let parsed = ref [] let nea = ref [] let parse file = thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; let i = open_in file in let startline = ref 0 in let fn = ref "" in let ids = ref [] in let rec inside n = let l = input_line i in let n = n + 1 in match Str.split_delim (Str.regexp_string "*/") l with before::after::_ -> (if not (!fn = "") then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); startline := 0; fn := ""; ids := []; outside n | _ -> (match Str.split (Str.regexp "[ \t]+") l with "*"::name::rest -> let len = String.length name in (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" then fn := String.sub name 0 (len-2) else if !fn = "" && (not (rest = [])) && List.hd rest = "-" then if String.get name (len-1) = ':' then fn := String.sub name 0 (len-1) else fn := name else if not(!fn = "") && len > 2 && String.get name 0 = '@' && String.get name (len-1) = ':' then ids := (String.sub name 1 (len-2)) :: !ids); | _ -> ()); inside n and outside n = let l = input_line i in let n = n + 1 in if String.length l > 2 && String.sub l 0 3 = "/**" then begin startline := n; inside n end else outside n in try outside 0 with End_of_file -> () let hashadd tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref [] in Hashtbl.add tbl k cell; cell in cell := v :: !cell @script:ocaml@ @@ tbl := []; fnstart := []; Hashtbl.clear success; parsed := []; nea := []; parse (List.hd (Coccilib.files())) @r@ identifier f; position p; @@ f@p(...) { ... } @script:ocaml@ p << r.p; f << r.f; @@ parsed := f :: !parsed; fnstart := (List.hd p).line :: !fnstart @param@ identifier f; type T; identifier i; parameter list[n] ps; parameter list[n1] ps1; position p; @@ f@p(ps,T i,ps1) { ... } @script:ocaml@ @@ tbl := List.rev (List.sort compare !tbl) @script:ocaml@ p << param.p; f << param.f; @@ let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if not (String.lowercase f = String.lowercase nm) then Printf.printf "%s:%d %s doesn't match preceding comment: %s\n" !thefile myline f nm); true end else false) !tbl in () @script:ocaml@ p << param.p; n << param.n; n1 << param.n1; i << param.i; f << param.f; @@ let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if List.mem i ids then hashadd success (st,fn,nm) i); (if ids = [] (* arg list seems not obligatory *) then () else if not (List.mem i ids) then Printf.printf "%s:%d %s doesn't appear in ids: %s\n" !thefile myline i (String.concat " " ids) else if List.length ids <= n || List.length ids <= n1 then (if not (List.mem f !nea) then begin nea := f :: !nea; Printf.printf "%s:%d %s not enough args\n" !thefile myline f; end) else let foundid = List.nth ids n in let efoundid = List.nth (List.rev ids) n1 in if not(foundid = i || efoundid = i) then Printf.printf "%s:%d %s wrong arg in position %d: %s\n" !thefile myline i n foundid); true end else false) !tbl in () @script:ocaml@ @@ List.iter (function (st,fn,nm,ids) -> if List.mem nm !parsed then let entry = try !(Hashtbl.find success (st,fn,nm)) with Not_found -> [] in List.iter (fun id -> if not (List.mem id entry) && not (id = "...") then Printf.printf "%s:%d %s not used\n" !thefile st id) ids) !tbl // </smpl> --- drivers/clk/keystone/pll.c | 4 ++-- drivers/clk/sunxi/clk-mod0.c | 2 +- drivers/clk/tegra/cvb.c | 10 +++++----- drivers/dma-buf/sw_sync.c | 6 +++--- drivers/gpu/drm/gma500/intel_i2c.c | 3 +-- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- drivers/irqchip/irq-metag-ext.c | 1 - drivers/irqchip/irq-vic.c | 1 - drivers/mfd/tc3589x.c | 4 ++-- drivers/power/supply/ab8500_fg.c | 8 ++++---- drivers/power/supply/abx500_chargalg.c | 1 + drivers/power/supply/intel_mid_battery.c | 2 +- drivers/power/supply/power_supply_core.c | 4 ++-- fs/crypto/crypto.c | 4 ++-- fs/crypto/fname.c | 4 ++-- fs/ubifs/file.c | 2 +- fs/ubifs/gc.c | 2 +- fs/ubifs/lprops.c | 2 +- fs/ubifs/lpt_commit.c | 4 +--- fs/ubifs/replay.c | 2 +- lib/kobject_uevent.c | 6 +++--- lib/lru_cache.c | 4 ++-- lib/nlattr.c | 2 +- 23 files changed, 39 insertions(+), 43 deletions(-)

7 years, 6 months

Re: [Linaro-mm-sig] [PATCH 00/15] improve function-level documentation

by Julia Lawall

On Sat, 1 Oct 2016, Joe Perches wrote: > On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote: > > These patches fix cases where the documentation above a function definition > > is not consistent with the function header. Issues are detected using the > > semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic > > patch parses a file to find comments, then matches each function header, > > and checks that the name and parameter list in the function header are > > compatible with the comment that preceeds it most closely. > > Hi Julia. > > Would it be possible for a semantic patch to scan for > function definitions where the types do not have > identifiers and update the definitions to match the > declarations? > > For instance, given: > > <some.h> > int foo(int); > > <some.c> > int foo(int bar) > { > return baz; > } > > Could coccinelle output: > > diff a/some.h b/some.h > [] > -int foo(int); > +int foo(int bar); The following seems to work: @r@ identifier f; position p; type T, t; parameter list[n] ps; @@ T f@p(ps,t,...); @s@ identifier r.f,x; type r.T, r.t; parameter list[r.n] ps; @@ T f(ps,t x,...) { ... } @@ identifier r.f, s.x; position r.p; type r.T, r.t; parameter list[r.n] ps; @@ T f@p(ps,t + x ,...); After letting it run for a few minutes without making any effort to include .h files, I get over 2700 changed lines. julia

7 years, 6 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig October 2016