Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

From: Dan Williams <dan.j.williams@intel.com>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>,
	"Williams, Dan J" <dan.j.williams@intel.com>
Cc: "Schofield, Alison" <alison.schofield@intel.com>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands
Date: Wed, 13 Jul 2022 09:55:05 -0700	[thread overview]
Message-ID: <62cef8e9e8ac6_156c6529454@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <e0e83c8e3a45b432ed1747b0b8442a7a11db8579.camel@intel.com>

Verma, Vishal L wrote:
> On Wed, 2022-07-13 at 08:22 -0700, Dan Williams wrote:
> > Verma, Vishal L wrote:
> > > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > > 
> > > > 
> > > > +
> > > > +       if (cxl_decoder_get_mode(target) != mode) {
> > > > +               rc = cxl_decoder_set_dpa_size(target, 0);
> > > > +               if (rc) {
> > > > +                       log_err(&ml,
> > > > +                               "%s: %s: failed to clear allocation to set mode\n",
> > > > +                               devname, cxl_decoder_get_devname(target));
> > > > +                       return rc;
> > > > +               }
> > > > +               rc = cxl_decoder_set_mode(target, mode);
> > > > +               if (rc) {
> > > > +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> > > > +                               cxl_decoder_get_devname(target),
> > > > +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> > > > +                       return rc;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       rc = cxl_decoder_set_dpa_size(target, size);
> > > > +       if (rc)
> > > > +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,
> > > 
> > > This patch adds some >80 col lines, which is fine by me - maybe we
> > > should update .clang-format to 100 to make it official?
> > 
> > .clang_format does not break up print format strings that span 80
> > columns. Same as the kernel. So those are properly formatted as the
> > non-format string portions of those print statements stay <= 80.
> 
> Oh sure - though I thought it would at least drop the " devname" to the
> next line. (Just checked, looked like it does).

Of course I only checked the 2 previous log_err() in the quote and not
the one you directly commented on, will reflow.

> 
> There were some other non-string long lines as well, e.g.:
> 
> +static int action_reserve_dpa(struct cxl_memdev *memdev, struct action_context *actx)

Ok, will reflow that as well.