On Thu, 7 Sep 2023 11:41:11 +0200 Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > On 9/7/23 10:42, Boris Brezillon wrote: > > On Wed, 6 Sep 2023 23:47:13 +0200 > > Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > > >> +void drm_gpuvm_bo_destroy(struct kref *kref); > > > > I usually keep kref's release functions private so people are not > > tempted to use them. > > I think I did that because drm_gpuvm_bo_put() needs it. Yeah, but you can move the drm_gpuvm_bo_put() implementation to the C file and make this one private. > > > > >> + > >> +/** > >> + * drm_gpuvm_bo_get() - acquire a struct drm_gpuvm_bo reference > >> + * @vm_bo: the &drm_gpuvm_bo to acquire the reference of > >> + * > >> + * This function acquires an additional reference to @vm_bo. It is illegal to > >> + * call this without already holding a reference. No locks required. > >> + */ > >> +static inline struct drm_gpuvm_bo * > >> +drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo) > >> +{ > >> + kref_get(&vm_bo->kref); > >> + return vm_bo; > >> +} > >> + > >> +/** > >> + * drm_gpuvm_bo_put() - drop a struct drm_gpuvm_bo reference > >> + * @vm_bo: the &drm_gpuvm_bo to release the reference of > >> + * > >> + * This releases a reference to @vm_bo. > >> + */ > >> +static inline void > >> +drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo) > >> +{ > >> + kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy); > > > > nit: can we have > > > > if (vm_bo) > > kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy); > > > > so we don't have to bother testing the vm_bo value in the error/cleanup > > paths? > > > >> +} > >> + > > >