From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6EB74B652 for ; Thu, 25 Dec 2025 18:13:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766686399; cv=none; b=YmQEK6pRbtR9x6TPHlxj9H0HbMW3oLXTubdBIUenqkeX+6cERDmHghO36sTmsuCiwsePa0WFCZ9iAYTzV2cjanKtWg+fmyC1ZL0e9kWk1C2NCHfhq+P3EbKT3dXqU+C0XhMApLZbnrreSK2bL5LSPdq/r75EB4r0j+ZacFskjWk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766686399; c=relaxed/simple; bh=jizeCBAZZoAuSU9UhQCCqNxwaeMskUgqByHijedloNc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BddEVOYEwPXnD4m2cMcGQGGFPYyUNNVQl8NBUI1s7pz0Y170lK/ABXjeIgHLPEUEFlo1DM6O+rX4yTyT8sNXYPRXhRJlzvnFj937V+BWFrRYvqOdSzWss5gw/hR03zEWo6rhALtDfE3/7zzjdD91QiRN9p9NMqkbcp4H/n4SvCU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=purestorage.com; spf=fail smtp.mailfrom=purestorage.com; dkim=pass (2048-bit key) header.d=purestorage.com header.i=@purestorage.com header.b=Tlo/fRxv; arc=none smtp.client-ip=209.85.215.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=purestorage.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=purestorage.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=purestorage.com header.i=@purestorage.com header.b="Tlo/fRxv" Received: by mail-pg1-f178.google.com with SMTP id 41be03b00d2f7-b4755f37c3eso4403870a12.3 for ; Thu, 25 Dec 2025 10:13:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google2022; t=1766686396; x=1767291196; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=6hgD7o3iJ2HUveiHrqiwWb4S5cgYHnaYDUDf9EQETEc=; b=Tlo/fRxv+MNbFmxmm0AF8fKPQ1SIlwd0aT1G+wy5NWKKbng9tLwfoW2bpNipfcw3it K1h7oa9TD5WnswwVFJ4AjYTvVXypuNS8pfXw+Qu9acXe6go+iLJ44t96ugbteEmwkSNc Fy+FR/QIXytK/UCqEaCmdzqIfvEQukQRErpSye00rXUwjNp32doPbPWasMlWeRgf/044 7IYsGVnPxSynroul1WOI3+CjpA911g5SjS1VbRguyOFBsEhctStKKasZ0BxT+ef6/8l1 1iWu0UNbKyPvNOfhTX2zxhKgRK1pEzVg11UBX8z0H1BXzfU9LmN2fB+8k5T0JRcMxSyd jgTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766686396; x=1767291196; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6hgD7o3iJ2HUveiHrqiwWb4S5cgYHnaYDUDf9EQETEc=; b=J8RFFTe3o1+G6C+KV1KEi/MAsGfHqH+UDVF0p4PWs8Cqyna0PgsbcGwm2nRR93/BxC Q+1XrznM6O9KbBXyYzGzhyDjVjiYXnZE2uzwDf+NhgQKerXmrL/v9MBLdZh62o5lfUjC GkW0I1ScAy+0g04acew+P1KM4EZ1PIg9d7lhdy2DY1qJriDQdd6zXOtOnWOLzfoRJN0U D5cB8aSr33xVDK36Jn5wc9bBa2M4RLrn+oHlI/4IbJ9MDfLUsgd6gznRfP+Op/UhaGGJ qegOpAJhamzJlCBGkp35c9KKU4rPeVKg7M+32RdUzaD/+KZz474wmdO1lgy6jp564z09 7MMg== X-Forwarded-Encrypted: i=1; AJvYcCUI5FKi2qiVPbBr995MTBiowjhbwLGaOnLvO0MlkH+MDm6EbYWRLCML80SpdrxwZJoeTmnyNOZX5zryahA=@vger.kernel.org X-Gm-Message-State: AOJu0Ywn454NuMjJH8M2MtEvllTQ5YaqmLjbEpneYbr/tQkHJpRE0WeT nRGY+Kat/bAFwSNSbqVNfFOouqvrO3jiL0RcqL/mQ938FGnriUygkaAlmiZ4siOaowI= X-Gm-Gg: AY/fxX55sFktPTbcXsKEdWLO9cLJzvcZ+z47lUvpxkZApxe6+FBSlNJfDbWMI1UdlEa gV2Pa1yBEQb4A0ZX1qXYigGqZEI09S/fx6/gCbufjFqFuimXHBU3HMBXRRpmDayJ/T/tjz0QqPi V+5P6rgXkVCLS8TAQEH6SUhFtsxurvSHA5GOO7CUCfDnIZXptem62tid8VjFvt/ZMK2wF0Qzc2t 7pAfVhj+RjT8SuIGbhGgX5DJyXNUcyRSaScDfbbp73DSwx09ZlyWEiibQGwydMYTVRgB7e/qU9D EzgNJyamUopHRrR+SYR5S5iTR0EOl/fsMklRtyBvzmh2oua22SrGbulpvILZqdpZWsu65rUIexb oGGnt5g39Bol9xHGZoTt5Sr04QP9wtv2L3E6kCZmAqhlnbiQrBHw3FkrCK7lI5rjkjPAcCSCm62 h06e/NKntq5QNM2kg= X-Google-Smtp-Source: AGHT+IGg/8ZqY1GLplovc57sq7jXcZokEvESLzzFd8J3CScL0LLjIaQVQtIE5xKF/k84Sv74zPO3Gg== X-Received: by 2002:a05:7022:6291:b0:119:e569:f61e with SMTP id a92af1059eb24-121722e12e7mr18317369c88.23.1766686395398; Thu, 25 Dec 2025 10:13:15 -0800 (PST) Received: from medusa.lab.kspace.sh ([2601:640:8202:6fb0::1305]) by smtp.googlemail.com with UTF8SMTPSA id a92af1059eb24-121724ddc30sm80960225c88.6.2025.12.25.10.13.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Dec 2025 10:13:15 -0800 (PST) Date: Thu, 25 Dec 2025 10:13:13 -0800 From: Mohamed Khalfella To: Sagi Grimberg Cc: Chaitanya Kulkarni , Christoph Hellwig , Jens Axboe , Keith Busch , Aaron Dailey , Randy Jennings , John Meneghini , Hannes Reinecke , linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 05/14] nvmet: Send an AEN on CCR completion Message-ID: <20251225181313.GC8129-mkhalfella@purestorage.com> References: <20251126021250.2583630-1-mkhalfella@purestorage.com> <20251126021250.2583630-6-mkhalfella@purestorage.com> <13c2bf9c-6eac-4c48-b12c-f76e86c8ef32@grimberg.me> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <13c2bf9c-6eac-4c48-b12c-f76e86c8ef32@grimberg.me> On Thu 2025-12-25 15:23:51 +0200, Sagi Grimberg wrote: > > > On 26/11/2025 4:11, Mohamed Khalfella wrote: > > Send an AEN to initiator when impacted controller exists. The > > notification points to CCR log page that initiator can read to check > > which CCR operation completed. > > > > Signed-off-by: Mohamed Khalfella > > --- > > drivers/nvme/target/core.c | 27 +++++++++++++++++++++++---- > > drivers/nvme/target/nvmet.h | 3 ++- > > include/linux/nvme.h | 3 +++ > > 3 files changed, 28 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c > > index 7dbe9255ff42..60173833c3eb 100644 > > --- a/drivers/nvme/target/core.c > > +++ b/drivers/nvme/target/core.c > > @@ -202,7 +202,7 @@ static void nvmet_async_event_work(struct work_struct *work) > > nvmet_async_events_process(ctrl); > > } > > > > -void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, > > +static void nvmet_add_async_event_locked(struct nvmet_ctrl *ctrl, u8 event_type, > > u8 event_info, u8 log_page) > > { > > struct nvmet_async_event *aen; > > @@ -215,12 +215,17 @@ void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, > > aen->event_info = event_info; > > aen->log_page = log_page; > > > > - mutex_lock(&ctrl->lock); > > list_add_tail(&aen->entry, &ctrl->async_events); > > - mutex_unlock(&ctrl->lock); > > > > queue_work(nvmet_wq, &ctrl->async_event_work); > > } > > +void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, > > + u8 event_info, u8 log_page) > > +{ > > + mutex_lock(&ctrl->lock); > > + nvmet_add_async_event_locked(ctrl, event_type, event_info, log_page); > > + mutex_unlock(&ctrl->lock); > > +} > > > > static void nvmet_add_to_changed_ns_log(struct nvmet_ctrl *ctrl, __le32 nsid) > > { > > @@ -1788,6 +1793,18 @@ struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args) > > } > > EXPORT_SYMBOL_GPL(nvmet_alloc_ctrl); > > > > +static void nvmet_ctrl_notify_ccr(struct nvmet_ctrl *ctrl) > > +{ > > + lockdep_assert_held(&ctrl->lock); > > + > > + if (nvmet_aen_bit_disabled(ctrl, NVME_AEN_BIT_CCR_COMPLETE)) > > + return; > > + > > + nvmet_add_async_event_locked(ctrl, NVME_AER_NOTICE, > > + NVME_AER_NOTICE_CCR_COMPLETED, > > + NVME_LOG_CCR); > > +} > > + > > static void nvmet_ctrl_complete_pending_ccr(struct nvmet_ctrl *ctrl) > > { > > struct nvmet_subsys *subsys = ctrl->subsys; > > @@ -1801,8 +1818,10 @@ static void nvmet_ctrl_complete_pending_ccr(struct nvmet_ctrl *ctrl) > > list_for_each_entry(sctrl, &subsys->ctrls, subsys_entry) { > > mutex_lock(&sctrl->lock); > > list_for_each_entry(ccr, &sctrl->ccrs, entry) { > > - if (ccr->ctrl == ctrl) > > + if (ccr->ctrl == ctrl) { > > + nvmet_ctrl_notify_ccr(sctrl); > > ccr->ctrl = NULL; > > + } > > Is this double loop necessary? Would you have more than one controller > cross resetting the same As it is implemented now CCRs are linked to sctrl. This decision can be revisited if found suboptimal. At some point I had CCRs linked to ctrl->subsys but that led to lock ordering issues. Double loop is necessary to find all CCRs in all controllers and mark them done. Yes, it is possible to have more than one sctrl resetting the same ictrl. > controller? Won't it be better to install a callback+opaque that the > controller removal will call? Can you elaborate more on that? Better in what terms? nvmet_ctrl_complete_pending_ccr() is called from nvmet_ctrl_free() when we know that ctrl->ref is zero and no new CCRs will be added to this controller because nvmet_ctrl_find_get_ccr() will not be able to get it.