[{"id":32663,"web_url":"https://patchwork.libcamera.org/comment/32663/","msgid":"<173385152028.3661019.15139530884759440635@ping.linuxembedded.co.uk>","date":"2024-12-10T17:25:20","subject":"Re: [PATCH 1/1] DmaBufAllocator: Make DmaSyncer non-copyable","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Harvey Yang (2024-11-29 18:53:28)\n> This fixes commit 39482d59fe7160740ea5dd61f3ed965a88d848ce.\n\nHave you seen how we style fixes tags to match the linux kernel?\n\nTaken from: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes\n\n\n\"\"\"\n\nIf your patch fixes a bug in a specific commit, e.g. you found an issue\nusing git bisect, please use the ‘Fixes:’ tag with the first 12\ncharacters of the SHA-1 ID, and the one line summary. Do not split the\ntag across multiple lines, tags are exempt from the “wrap at 75 columns”\nrule in order to simplify parsing scripts. For example:\n\nFixes: 54a4f0239f2e (\"KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed\")\n\nThe following git config settings can be used to add a pretty format for\noutputting the above style in the git log or git show commands:\n\n[core]\n abbrev = 12\n[pretty]\n fixes = Fixes: %h (\\\"%s\\\")\n\nAn example call:\n\n$ git log -1 --pretty=fixes 54a4f0239f2e\nFixes: 54a4f0239f2e (\"KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed\")\n\n\"\"\"\n\n\n\n> \n> As DmaSyncer does sync start/end in the c'tor/d'tor, copying a DmaSyncer\n> instance would trigger sync end earlier than expected. This patch makes\n> it non-copyable to avoid the issue.\n> \n> Signed-off-by: Harvey Yang \n\nWith the fixes tag corrected\n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> include/libcamera/internal/dma_buf_allocator.h | 5 +++++\n> src/libcamera/dma_buf_allocator.cpp | 10 ++++++++++\n> 2 files changed, 15 insertions(+)\n> \n> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> index fc5de2c13..0cb209630 100644\n> --- a/include/libcamera/internal/dma_buf_allocator.h\n> +++ b/include/libcamera/internal/dma_buf_allocator.h\n> @@ -60,9 +60,14 @@ public:\n> \n> explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> \n> + DmaSyncer(DmaSyncer &&) = default;\n> + DmaSyncer &operator=(DmaSyncer &&) = default;\n> +\n> ~DmaSyncer();\n> \n> private:\n> + LIBCAMERA_DISABLE_COPY(DmaSyncer)\n> +\n> void sync(uint64_t step);\n> \n> SharedFD fd_;\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index 3cc52f968..5f517828d 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -311,6 +311,16 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> sync(DMA_BUF_SYNC_START);\n> }\n> \n> +/**\n> + * \\fn DmaSyncer::DmaSyncer(DmaSyncer &&) = default;\n> + * \\brief Enable move on class DmaSyncer\n> + */\n> +\n> +/**\n> + * \\fn DmaSyncer::operator=(DmaSyncer &&) = default;\n> + * \\brief Enable move on class DmaSyncer\n> + */\n> +\n> DmaSyncer::~DmaSyncer()\n> {\n> sync(DMA_BUF_SYNC_END);\n> -- \n> 2.47.0.338.g60cca15819-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 43593BD80A\n\tfor ;\n\tTue, 10 Dec 2024 17:25:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 451D867E94;\n\tTue, 10 Dec 2024 18:25:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6081C618AF\n\tfor ;\n\tTue, 10 Dec 2024 18:25:23 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B46AD6EC;\n\tTue, 10 Dec 2024 18:24:50 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"HK3TFkbT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733851490;\n\tbh=EPC6KOCEgvSI4+bbiXtu63PJQo+xwuxOp+iT+yT2V2M=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=HK3TFkbTWd8EsDrMN60GSKicCDDHpeftRAkhEAIa1psxGoqv84HbV/A6yzbI6Kqf8\n\tXD2BBO/Rnoq3PD/kxuGoKwuHGNgcZd0KLMvYI/jrFpgVniw4xYr3UcS2XrVONsu4PW\n\tMwjdwfH9K+IRFNY8zKKsTbCndK+QqEAQX9tIiHz4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241129185533.3308449-2-chenghaoyang@chromium.org>","References":"<20241129185533.3308449-1-chenghaoyang@chromium.org>\n\t<20241129185533.3308449-2-chenghaoyang@chromium.org>","Subject":"Re: [PATCH 1/1] DmaBufAllocator: Make DmaSyncer non-copyable","From":"Kieran Bingham ","Cc":"Harvey Yang ","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 10 Dec 2024 17:25:20 +0000","Message-ID":"<173385152028.3661019.15139530884759440635@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32665,"web_url":"https://patchwork.libcamera.org/comment/32665/","msgid":"","date":"2024-12-11T04:17:13","subject":"Re: [PATCH 1/1] DmaBufAllocator: Make DmaSyncer non-copyable","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Wed, Dec 11, 2024 at 1:25 AM Kieran Bingham\n wrote:\n>\n> Quoting Harvey Yang (2024-11-29 18:53:28)\n> > This fixes commit 39482d59fe7160740ea5dd61f3ed965a88d848ce.\n>\n> Have you seen how we style fixes tags to match the linux kernel?\n>\n> Taken from: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes\n>\n>\n> \"\"\"\n>\n> If your patch fixes a bug in a specific commit, e.g. you found an issue\n> using git bisect, please use the ‘Fixes:’ tag with the first 12\n> characters of the SHA-1 ID, and the one line summary. Do not split the\n> tag across multiple lines, tags are exempt from the “wrap at 75 columns”\n> rule in order to simplify parsing scripts. For example:\n>\n> Fixes: 54a4f0239f2e (\"KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed\")\n>\n> The following git config settings can be used to add a pretty format for\n> outputting the above style in the git log or git show commands:\n>\n> [core]\n> abbrev = 12\n> [pretty]\n> fixes = Fixes: %h (\\\"%s\\\")\n>\n> An example call:\n>\n> $ git log -1 --pretty=fixes 54a4f0239f2e\n> Fixes: 54a4f0239f2e (\"KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed\")\n>\n\nThanks for the suggestion!\nAdded in my git config, and sent a new version.\n\nBR,\nHarvey\n\n> \"\"\"\n>\n>\n>\n> >\n> > As DmaSyncer does sync start/end in the c'tor/d'tor, copying a DmaSyncer\n> > instance would trigger sync end earlier than expected. This patch makes\n> > it non-copyable to avoid the issue.\n> >\n> > Signed-off-by: Harvey Yang \n>\n> With the fixes tag corrected\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> > ---\n> > include/libcamera/internal/dma_buf_allocator.h | 5 +++++\n> > src/libcamera/dma_buf_allocator.cpp | 10 ++++++++++\n> > 2 files changed, 15 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h\n> > index fc5de2c13..0cb209630 100644\n> > --- a/include/libcamera/internal/dma_buf_allocator.h\n> > +++ b/include/libcamera/internal/dma_buf_allocator.h\n> > @@ -60,9 +60,14 @@ public:\n> >\n> > explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);\n> >\n> > + DmaSyncer(DmaSyncer &&) = default;\n> > + DmaSyncer &operator=(DmaSyncer &&) = default;\n> > +\n> > ~DmaSyncer();\n> >\n> > private:\n> > + LIBCAMERA_DISABLE_COPY(DmaSyncer)\n> > +\n> > void sync(uint64_t step);\n> >\n> > SharedFD fd_;\n> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> > index 3cc52f968..5f517828d 100644\n> > --- a/src/libcamera/dma_buf_allocator.cpp\n> > +++ b/src/libcamera/dma_buf_allocator.cpp\n> > @@ -311,6 +311,16 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)\n> > sync(DMA_BUF_SYNC_START);\n> > }\n> >\n> > +/**\n> > + * \\fn DmaSyncer::DmaSyncer(DmaSyncer &&) = default;\n> > + * \\brief Enable move on class DmaSyncer\n> > + */\n> > +\n> > +/**\n> > + * \\fn DmaSyncer::operator=(DmaSyncer &&) = default;\n> > + * \\brief Enable move on class DmaSyncer\n> > + */\n> > +\n> > DmaSyncer::~DmaSyncer()\n> > {\n> > sync(DMA_BUF_SYNC_END);\n> > --\n> > 2.47.0.338.g60cca15819-goog\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C86B0BD80A\n\tfor ;\n\tWed, 11 Dec 2024 04:17:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CA1D67E9E;\n\tWed, 11 Dec 2024 05:17:26 +0100 (CET)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0D1A618AA\n\tfor ;\n\tWed, 11 Dec 2024 05:17:24 +0100 (CET)","by mail-lj1-x22c.google.com with SMTP id\n\t38308e7fff4ca-2ff976ab0edso58248131fa.1\n\tfor ;\n\tTue, 10 Dec 2024 20:17:24 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"YoP/+Wun\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1733890644; x=1734495444;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=U9V6Wr2b/6FcDMR6Nd1fkg/JAO0OasxT29huvk4trqI=;\n\tb=YoP/+Wun0flgyINlGqZ0esbZr16gxK9fYYQqvdtEHyPThxKUUQxXN/r0OvRoc+PjvX\n\tCx+7bKhSatjUbONXu2UgPUCo6KMG1XCjtA5W0xIeyArhv+qjrbgT5pgwMm3oouAn+w9n\n\tfkKetYGVXM5EVHmzNvXIsZXDOMpzrhT2trSfw=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733890644; x=1734495444;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=U9V6Wr2b/6FcDMR6Nd1fkg/JAO0OasxT29huvk4trqI=;\n\tb=cqMVxI4gWGP5Xf9MONsXpzdA7uLmR7PVyDCzKJ1IeWrTVwm+69md9ZWN0PSZ8IYOrj\n\tylyDID/VquIsu4KcEVgcEQy5APS6COUFpBNuthKdiW0GZE1nXq2d5TNOS5VJVUtPY033\n\t2yHtcAkur6ZOyqCj5zSCjyHirR919PEz/loY/b+ZBXPp2Efsj0SYQINvrzZA1ibX1A0p\n\tYVQ2DsNVvEpTVfFJH0yIkyfw+l5D1JV6dDAm7CqSOGe1l+32SeqpFhtCeoo0FaG44Yh4\n\tfz/lPeyjY3K1BOKz9AETRw9dZPoB+Ijino49H/rU2V3Orc2Sh4kEEnVjKxeY6cnWVpbe\n\tZZnQ==","X-Gm-Message-State":"AOJu0Yzk5rNrPvlybK+T9gbLnRD1KXiVLNfk5wZCbA76QvO4IWHEZPGp\n\tC+rJwL5tc66gcZp/kIpvb5UaWTixsj4uxIbfyfPf1mark3cXtmLsHS2dIhstgXqQYIcpgA9mzjI\n\tQB0GDWp+GcwqDLL2p9KLRSAMjLvcwt31EJVpV","X-Gm-Gg":"ASbGncvx/zz53/OgvXWHWLpMFG28F85zxBDiVfGvWo4+XHApzn2vYVv6ynwWss+kpCt\n\tvfQC97vLb0w2QD3gR1qq7JH2/MvPNOrgeO0cHvKVIYRJ7CmOe/badw8hReefo","X-Google-Smtp-Source":"AGHT+IGzysJJw1unbcN+Fmn0RG34nIzjLWF6u0N25xTZRlwDbMQig2D8FNBZduv8s4nB9Xu2t/Aq9Oi7KgywztnhBHs=","X-Received":"by 2002:a05:651c:b25:b0:302:1dd5:6b34 with SMTP id\n\t38308e7fff4ca-30240d5488emr5460971fa.29.1733890643939;\n\tTue, 10 Dec 2024 20:17:23 -0800 (PST)","MIME-Version":"1.0","References":"<20241129185533.3308449-1-chenghaoyang@chromium.org>\n\t<20241129185533.3308449-2-chenghaoyang@chromium.org>\n\t<173385152028.3661019.15139530884759440635@ping.linuxembedded.co.uk>","In-Reply-To":"<173385152028.3661019.15139530884759440635@ping.linuxembedded.co.uk>","From":"Cheng-Hao Yang ","Date":"Wed, 11 Dec 2024 12:17:13 +0800","Message-ID":"","Subject":"Re: [PATCH 1/1] DmaBufAllocator: Make DmaSyncer non-copyable","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]