public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jonathan.cameron@huawei.com>
To: Neeraj Kumar <s.neeraj@samsung.com>
Cc: Dave Jiang <dave.jiang@intel.com>, <linux-cxl@vger.kernel.org>,
	<nvdimm@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
	<gost.dev@samsung.com>, <a.manzanares@samsung.com>,
	<vishak.g@samsung.com>, <neeraj.kernel@gmail.com>,
	<cpgs@samsung.com>
Subject: Re: [PATCH V2 05/20] nvdimm/region_label: Add region label updation routine
Date: Wed, 10 Sep 2025 15:03:22 +0100	[thread overview]
Message-ID: <20250910150322.00001ea4@huawei.com> (raw)
In-Reply-To: <158453976.61757055783236.JavaMail.epsvc@epcpadp2new>

On Thu, 4 Sep 2025 19:42:31 +0530
Neeraj Kumar <s.neeraj@samsung.com> wrote:

> On 15/08/25 02:55PM, Dave Jiang wrote:
> >
> >
> >On 7/30/25 5:11 AM, Neeraj Kumar wrote:  
> >> Added __pmem_region_label_update region label update routine to update
> >> region label.
> >>
> >> Also used guard(mutex)(&nd_mapping->lock) in place of mutex_lock() and
> >> mutex_unlock()
> >>
> >> Signed-off-by: Neeraj Kumar <s.neeraj@samsung.com>  
> >
> >Subject, s/updation/update/ ?  
> 
> Thanks Dave, Sure. Will fix it in next patch-set
> 

Hi Neeraj,
 
Really small process point.  We all get too many emails, so
when replying crop out anything that doesn't need more discussion.
Where you are just saying you agree, leave that for the change logs of the
next version (and appropriate thanks can go there as well).

I know not replying can feel little rude, but trust me when I say all
or almost all who review a lot appreciate efficiency!

It also makes it a lot harder to miss the more substantial replies.

> >> +int nd_pmem_region_label_update(struct nd_region *nd_region)
> >> +{
> >> +	int i, rc;
> >> +
> >> +	for (i = 0; i < nd_region->ndr_mappings; i++) {
> >> +		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
> >> +		struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
> >> +
> >> +		/* No need to update region label for non cxl format */
> >> +		if (!ndd->cxl)
> >> +			continue;  
> >
> >Would there be a mix of different nd mappings? I wonder if you can just 'return 0' if you find ndd->cxl on the first one and just skip everything.  
> 
> When we create cxl region with two mem device, then we will have two separate
> nd_mapping for both mem devices. But Yes, I don't see difference in both device
> nd_mapping characters. So instead of "continue", I will just "return 0".
> 

This for instance is good discussion and well worth the reply!

Good discussion is great, but reducing the noise is key to keeping
things manageable.

Jonathan

p.s. I'm having a full day or reviewing code so getting a little grumpier
than I was first thing this morning when I might not have given this feedback!

  reply	other threads:[~2025-09-10 14:03 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20250730121221epcas5p3ffb9e643af6b8ae07cfccf0bdee90e37@epcas5p3.samsung.com>
2025-07-30 12:11 ` [PATCH V2 00/20] Add CXL LSA 2.1 format support in nvdimm and cxl pmem Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 01/20] nvdimm/label: Introduce NDD_CXL_LABEL flag to set cxl label format Neeraj Kumar
2025-08-13 13:12     ` Jonathan Cameron
2025-08-15 18:06       ` Dave Jiang
2025-09-04 13:24         ` Neeraj Kumar
2025-09-04 13:20       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 02/20] nvdimm/label: Prep patch to accommodate cxl lsa 2.1 support Neeraj Kumar
2025-08-13 13:23     ` Jonathan Cameron
2025-09-04 13:27       ` Neeraj Kumar
2025-08-15 22:02     ` Dave Jiang
2025-09-04 13:31       ` Neeraj Kumar
2025-08-18 21:48     ` Dave Jiang
2025-09-04 13:33       ` Neeraj Kumar
2025-08-19 15:38     ` Ira Weiny
2025-09-04 13:42       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 03/20] nvdimm/namespace_label: Add namespace label changes as per CXL LSA v2.1 Neeraj Kumar
2025-07-31 13:12     ` kernel test robot
2025-08-13 13:27     ` Jonathan Cameron
2025-09-04 13:40       ` Neeraj Kumar
2025-08-19 15:57     ` Ira Weiny
2025-09-04 13:51       ` Neeraj Kumar
2025-08-19 19:36     ` Ira Weiny
2025-09-05  5:34       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 04/20] nvdimm/label: CXL labels skip the need for 'interleave-set cookie' Neeraj Kumar
2025-08-13 13:44     ` Jonathan Cameron
2025-09-04 13:54       ` Neeraj Kumar
2025-08-15 21:02     ` Dave Jiang
2025-09-04 14:01       ` Neeraj Kumar
2025-08-19 16:04     ` Ira Weiny
2025-09-04 14:02       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 05/20] nvdimm/region_label: Add region label updation routine Neeraj Kumar
2025-07-31 15:07     ` kernel test robot
2025-08-13 14:48     ` Jonathan Cameron
2025-09-04 14:06       ` Neeraj Kumar
2025-08-15 21:55     ` Dave Jiang
2025-09-04 14:12       ` Neeraj Kumar
2025-09-10 14:03         ` Jonathan Cameron [this message]
2025-08-15 23:12     ` Dave Jiang
2025-09-04 14:13       ` Neeraj Kumar
2025-08-19 18:16     ` Ira Weiny
2025-09-04 14:18       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 06/20] nvdimm/region_label: Add region label deletion routine Neeraj Kumar
2025-08-13 14:53     ` Jonathan Cameron
2025-09-04 14:20       ` Neeraj Kumar
2025-08-15 22:22     ` Dave Jiang
2025-09-04 14:23       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 07/20] nvdimm/namespace_label: Update namespace init_labels and its region_uuid Neeraj Kumar
2025-08-13 14:58     ` Jonathan Cameron
2025-09-04 14:24       ` Neeraj Kumar
2025-08-19 18:56     ` Ira Weiny
2025-09-04 14:28       ` Neeraj Kumar
2025-09-05 20:08         ` Ira Weiny
2025-09-08  5:36           ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 08/20] nvdimm/label: Include region label in slot validation Neeraj Kumar
2025-08-13 15:07     ` Jonathan Cameron
2025-09-04 14:30       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 09/20] nvdimm/namespace_label: Skip region label during ns label DPA reservation Neeraj Kumar
2025-08-13 15:09     ` Jonathan Cameron
2025-09-04 14:31       ` Neeraj Kumar
2025-07-30 12:11   ` [PATCH V2 10/20] nvdimm/region_label: Preserve cxl region information from region label Neeraj Kumar
2025-08-13 15:11     ` Jonathan Cameron
2025-09-04 14:33       ` Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 11/20] nvdimm/region_label: Export routine to fetch region information Neeraj Kumar
2025-08-13 15:13     ` Jonathan Cameron
2025-07-30 12:12   ` [PATCH V2 12/20] nvdimm/namespace_label: Skip region label during namespace creation Neeraj Kumar
2025-08-13 15:55     ` Jonathan Cameron
2025-09-04 14:34       ` Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 13/20] cxl/mem: Refactor cxl pmem region auto-assembling Neeraj Kumar
2025-08-20 16:41     ` Dave Jiang
2025-09-04 14:39       ` Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 14/20] cxl/region: Add devm_cxl_pmem_add_region() for pmem region creation Neeraj Kumar
2025-08-20  0:30     ` Dave Jiang
2025-09-04 14:55       ` Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 15/20] cxl: Add a routine to find cxl root decoder on cxl bus using cxl port Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 16/20] cxl/mem: Preserve cxl root decoder during mem probe Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 17/20] cxl/pmem: Preserve region information into nd_set Neeraj Kumar
2025-07-30 12:12   ` [PATCH V2 18/20] cxl/pmem: Add support of cxl lsa 2.1 support in cxl pmem Neeraj Kumar
2025-07-31  1:36     ` kernel test robot
2025-07-30 12:12   ` [PATCH V2 19/20] cxl/pmem_region: Prep patch to accommodate pmem_region attributes Neeraj Kumar
2025-07-31  1:57     ` kernel test robot
2025-07-31  2:17     ` kernel test robot
2025-07-30 12:12   ` [PATCH V2 20/20] cxl/pmem_region: Add sysfs attribute cxl region label updation/deletion Neeraj Kumar
2025-07-31 10:36     ` kernel test robot
2025-08-07  9:02   ` [PATCH V2 00/20] Add CXL LSA 2.1 format support in nvdimm and cxl pmem Neeraj Kumar
2025-08-12 21:46     ` Dave Jiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250910150322.00001ea4@huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=a.manzanares@samsung.com \
    --cc=cpgs@samsung.com \
    --cc=dave.jiang@intel.com \
    --cc=gost.dev@samsung.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neeraj.kernel@gmail.com \
    --cc=nvdimm@lists.linux.dev \
    --cc=s.neeraj@samsung.com \
    --cc=vishak.g@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox