Message ID | 20250113202843.843466-1-chenghaoyang@chromium.org |
---|---|
State | Accepted |
Commit | 4dad8ece72c0c1c6c9e0101f4c31937bd739d001 |
Headers | show |
Series |
|
Related | show |
Harvey Yang <chenghaoyang@chromium.org> writes: > As DmaSyncer disables the copy c'tor, the move c'tor will be used > instead. This leaves some DmaSyncers with invalid SharedFDs. They should > avoid syncing with invalid file descriptors in the d'tor. > > Fixes: 545046a41e17 ("DmaBufAllocator: Make DmaSyncer non-copyable") > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > Tested-by: Milan Zamazal <mzamazal@redhat.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/dma_buf_allocator.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp > index a014c3b4263c..d8c62dd67a96 100644 > --- a/src/libcamera/dma_buf_allocator.cpp > +++ b/src/libcamera/dma_buf_allocator.cpp > @@ -325,7 +325,12 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type) > > DmaSyncer::~DmaSyncer() > { > - sync(DMA_BUF_SYNC_END); > + /* > + * DmaSyncer might be moved and left with an empty SharedFD. > + * Avoid syncing with an invalid file descriptor in this case. > + */ > + if (fd_.isValid()) > + sync(DMA_BUF_SYNC_END); > } > > void DmaSyncer::sync(uint64_t step)
Quoting Milan Zamazal (2025-01-14 09:10:12) > Harvey Yang <chenghaoyang@chromium.org> writes: > > > As DmaSyncer disables the copy c'tor, the move c'tor will be used > > instead. This leaves some DmaSyncers with invalid SharedFDs. They should > > avoid syncing with invalid file descriptors in the d'tor. > > > > Fixes: 545046a41e17 ("DmaBufAllocator: Make DmaSyncer non-copyable") > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > Tested-by: Milan Zamazal <mzamazal@redhat.com> > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Will push through CI and merge iff/when it completes. -- Kieran > > > --- > > src/libcamera/dma_buf_allocator.cpp | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp > > index a014c3b4263c..d8c62dd67a96 100644 > > --- a/src/libcamera/dma_buf_allocator.cpp > > +++ b/src/libcamera/dma_buf_allocator.cpp > > @@ -325,7 +325,12 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type) > > > > DmaSyncer::~DmaSyncer() > > { > > - sync(DMA_BUF_SYNC_END); > > + /* > > + * DmaSyncer might be moved and left with an empty SharedFD. > > + * Avoid syncing with an invalid file descriptor in this case. > > + */ > > + if (fd_.isValid()) > > + sync(DMA_BUF_SYNC_END); > > } > > > > void DmaSyncer::sync(uint64_t step) >
diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp index a014c3b4263c..d8c62dd67a96 100644 --- a/src/libcamera/dma_buf_allocator.cpp +++ b/src/libcamera/dma_buf_allocator.cpp @@ -325,7 +325,12 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type) DmaSyncer::~DmaSyncer() { - sync(DMA_BUF_SYNC_END); + /* + * DmaSyncer might be moved and left with an empty SharedFD. + * Avoid syncing with an invalid file descriptor in this case. + */ + if (fd_.isValid()) + sync(DMA_BUF_SYNC_END); } void DmaSyncer::sync(uint64_t step)