Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Trailing newlines in logging macros cause double newlines in the kernel log. --
--- Patch [5]: [PATCH v8 5/7] drm/tyr: add a kernel buffer object --- commit d331aa5f9ef1fb72722d9ab68ad7b2746d91080d Author: Deborah Brouwer <[email protected]> drm/tyr: add a kernel buffer object Introduce a buffer object type (KernelBo) for internal driver allocations that are managed by the kernel rather than userspace. It wraps a GEM shmem object and automatically handles GPU virtual address space mapping during creation and unmapping on drop. > diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs > index c28be61a01bb2..583c2a87b23b3 100644 > --- a/drivers/gpu/drm/tyr/gem.rs > +++ b/drivers/gpu/drm/tyr/gem.rs [ ... ] > +impl<'bound> KernelBo<'bound> { > + /// Creates a new kernel-owned buffer object and maps it into GPU VA > space. [ ... ] > + #[expect(dead_code)] > + pub(crate) fn new( > + ddev: &TyrDrmDevice, > + vm: Arc<Vm<'bound>>, > + size: u64, > + va_alloc: KernelBoVaAlloc, > + flags: VmMapFlags, > + ) -> Result<Self> { > + if size == 0 { > + dev_err!(vm.dev(), "Cannot create KernelBo with size 0\n"); [Severity: Low] This isn't a bug, but does this dev_err macro call need a trailing newline? The rust dev_err macro automatically appends a newline to the formatted output. Including an explicit newline at the end of the format string might result in an unnecessary blank line in the kernel log. > + return Err(EINVAL); > + } [ ... ] > +impl Drop for KernelBo<'_> { > + fn drop(&mut self) { > + let va = self.va_range.start; > + let size = self.va_range.end - self.va_range.start; > + > + if let Err(e) = self.vm.unmap_range(va, size) { > + // If unmap_range fails, it is still safe to drop the > + // KernelBo and its ARef to the GEM buffer object because > + // GPUVM also holds a reference to the GEM buffer object. > + // The physical pages won't be freed or reallocated. > + dev_err!( > + self.vm.dev(), > + "Failed to unmap KernelBo range {:#x}..{:#x}: {:?}\n", [Severity: Low] Similarly here, does this format string need the trailing newline? > + self.va_range.start, > + self.va_range.end, > + e > + ); > + } > + } > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
