linux-kernel - Re: [PATCH v7 4/7] scsi: mvsas: Add libsas SATA sysfs attributes group

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZemOSKWkpatQzNlt@ryzen>
Date: Thu, 7 Mar 2024 10:52:08 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Igor Pylypiv <ipylypiv@...gle.com>
Cc: Damien Le Moal <dlemoal@...nel.org>,
	John Garry <john.g.garry@...cle.com>,
	Jason Yan <yanaijie@...wei.com>,
	"James E.J. Bottomley" <jejb@...ux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	Jack Wang <jinpu.wang@...ud.ionos.com>,
	Hannes Reinecke <hare@...e.de>,
	Xiang Chen <chenxiang66@...ilicon.com>,
	Artur Paszkiewicz <artur.paszkiewicz@...el.com>,
	Bart Van Assche <bvanassche@....org>,
	TJ Adams <tadamsjr@...gle.com>, linux-ide@...r.kernel.org,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 4/7] scsi: mvsas: Add libsas SATA sysfs attributes
 group

On Wed, Mar 06, 2024 at 01:13:22PM -0800, Igor Pylypiv wrote:
> On Wed, Mar 06, 2024 at 11:55:19AM +0100, Niklas Cassel wrote:
> > On Tue, Mar 05, 2024 at 05:22:23PM -0800, Igor Pylypiv wrote:
> > > The added sysfs attributes group enables the configuration of NCQ Priority
> > > feature for HBAs that rely on libsas to manage SATA devices.
> > > 
> > > Reviewed-by: John Garry <john.g.garry@...cle.com>
> > > Reviewed-by: Damien Le Moal <dlemoal@...nel.org>
> > > Reviewed-by: Jason Yan <yanaijie@...wei.com>
> > > Signed-off-by: Igor Pylypiv <ipylypiv@...gle.com>
> > > ---
> > >  drivers/scsi/mvsas/mv_init.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> > > index 43ebb331e216..f1090bb5f2c9 100644
> > > --- a/drivers/scsi/mvsas/mv_init.c
> > > +++ b/drivers/scsi/mvsas/mv_init.c
> > > @@ -26,6 +26,7 @@ static const struct mvs_chip_info mvs_chips[] = {
> > >  };
> > >  
> > >  static const struct attribute_group *mvst_host_groups[];
> > > +static const struct attribute_group *mvst_sdev_groups[];
> > 
> > I think you can remove this line.
> >
> I kept the forward declaration to match the mvst_host_groups style.
> 
> Perhaps mvs_sht can be moved to the bottom of the file so that all forward
> declarations can be removed? This can be done in a separate cleanup patch
> series.
> 
> I'll keep this and aic94xx patches as-is, unless there are objections.

Usually, you first do the cleanup, then you do your changes.
(That way, there are fewer lines changed, since each patch is smaller.)

But no objection from me.


Kind regards,
Niklas


> 
> > 
> > >  
> > >  #define SOC_SAS_NUM 2
> > >  
> > > @@ -53,6 +54,7 @@ static const struct scsi_host_template mvs_sht = {
> > >  	.compat_ioctl		= sas_ioctl,
> > >  #endif
> > >  	.shost_groups		= mvst_host_groups,
> > > +	.sdev_groups		= mvst_sdev_groups,
> > >  	.track_queue_depth	= 1,
> > >  };
> > >  
> > > @@ -779,6 +781,11 @@ static struct attribute *mvst_host_attrs[] = {
> > >  
> > >  ATTRIBUTE_GROUPS(mvst_host);
> > >  
> > > +static const struct attribute_group *mvst_sdev_groups[] = {
> > > +	&sas_ata_sdev_attr_group,
> > > +	NULL
> > > +};
> > 
> > ..and move these lines up to be after:
> > static const struct attribute_group *mvst_host_groups[];
> > 
> > 
> > > +
> > >  module_init(mvs_init);
> > >  module_exit(mvs_exit);
> > >  
> > > -- 
> > > 2.44.0.278.ge034bb2e1d-goog
> > >