vhost: only emit debug log with invalid FD

Message ID 20240624144613.1855582-1-maxime.coquelin@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: only emit debug log with invalid FD |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Maxime Coquelin June 24, 2024, 2:46 p.m. UTC
This patch improves the FD manager logging in case
of FD removal from the epoll FD set failure.

When the FD is not more valid, like for example if it
has already been closed, only emit a debug log and not
an error one.

Fixes: 0e38b42bf61c ("vhost: manage FD with epoll")

Reported-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/fd_man.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
  

Comments

David Marchand June 25, 2024, 7:35 a.m. UTC | #1
On Mon, Jun 24, 2024 at 4:46 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch improves the FD manager logging in case
> of FD removal from the epoll FD set failure.
>
> When the FD is not more valid, like for example if it

is not valid anymore*

> has already been closed, only emit a debug log and not
> an error one.
>
> Fixes: 0e38b42bf61c ("vhost: manage FD with epoll")
>
> Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

I tested with OVS dpdk-latest and the patch lgtm.
Reviewed-by: David Marchand <david.marchand@redhat.com>
  
Maxime Coquelin June 25, 2024, 11:47 a.m. UTC | #2
On 6/25/24 09:35, David Marchand wrote:
> On Mon, Jun 24, 2024 at 4:46 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch improves the FD manager logging in case
>> of FD removal from the epoll FD set failure.
>>
>> When the FD is not more valid, like for example if it
> 
> is not valid anymore*
> 
>> has already been closed, only emit a debug log and not
>> an error one.
>>
>> Fixes: 0e38b42bf61c ("vhost: manage FD with epoll")
>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> I tested with OVS dpdk-latest and the patch lgtm.
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
> 

Applied to next-virtio/for-next-net with commit message amended with
your suggestion.

Thanks,
Maxime
  

Patch

diff --git a/lib/vhost/fd_man.c b/lib/vhost/fd_man.c
index 87a8dc3f3e..9bc7e50b93 100644
--- a/lib/vhost/fd_man.c
+++ b/lib/vhost/fd_man.c
@@ -256,9 +256,14 @@  fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat)
 static void
 fdset_del_locked(struct fdset *pfdset, struct fdentry *pfdentry)
 {
-	if (epoll_ctl(pfdset->epfd, EPOLL_CTL_DEL, pfdentry->fd, NULL) == -1)
-		VHOST_FDMAN_LOG(ERR, "could not remove %d fd from %d epfd: %s",
-			pfdentry->fd, pfdset->epfd, strerror(errno));
+	if (epoll_ctl(pfdset->epfd, EPOLL_CTL_DEL, pfdentry->fd, NULL) == -1) {
+		if (errno == EBADF) /* File might have already been closed. */
+			VHOST_FDMAN_LOG(DEBUG, "could not remove %d fd from %d epfd: %s",
+				pfdentry->fd, pfdset->epfd, strerror(errno));
+		else
+			VHOST_FDMAN_LOG(ERR, "could not remove %d fd from %d epfd: %s",
+				pfdentry->fd, pfdset->epfd, strerror(errno));
+	}
 
 	fdset_remove_entry(pfdset, pfdentry);
 }