Linux ATA/IDE development
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] libata: Fix ATA request sense
From: Tejun Heo @ 2017-01-06 20:44 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: linux-ide, Hannes Reinecke
In-Reply-To: <1482110260-14551-1-git-send-email-damien.lemoal@wdc.com>

On Mon, Dec 19, 2016 at 10:17:40AM +0900, Damien Le Moal wrote:
> For an ATA device supporting the sense data reporting feature set,
> a failed command will trigger the execution of ata_eh_request_sense if
> the result task file of the failed command has the ATA_SENSE bit set
> (sense data available bit). ata_eh_request_sense executes the
> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
> NOT be set (the command succeeded) but ata_eh_request_sense
> nevertheless tests the availability of sense data by testing that bit
> presence in the result tf of the REQUEST SENSE DATA EXT command.
> This leads to a falsy assume that request sense data failed and to the
> warning message:
> 
> atax.xx: request sense failed stat 50 emask 0
> 
> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
> result task file command so that sense data can be returned by
> ata_eh_request_sense.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Applied to libata/for-4.10-fixes with patch description updated as
suggested by Sergei.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [V1] ata: sata_mv:- Handle return value of devm_ioremap.
From: Tejun Heo @ 2017-01-06 20:46 UTC (permalink / raw)
  To: Arvind Yadav; +Cc: linux-ide, linux-kernel
In-Reply-To: <1481564607-9626-1-git-send-email-arvind.yadav.cs@gmail.com>

On Mon, Dec 12, 2016 at 11:13:27PM +0530, Arvind Yadav wrote:
> Here, If devm_ioremap will fail. It will return NULL.
> Then hpriv->base = NULL - 0x20000; Kernel can run into
> a NULL-pointer dereference. This error check will avoid
> NULL pointer dereference.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>

Applied to libata/for-4.10-fixes.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [V1] ata: sata_mv:- Handle return value of devm_ioremap.
From: Andy Shevchenko @ 2017-01-06 22:52 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Arvind Yadav, linux-ide, linux-kernel@vger.kernel.org
In-Reply-To: <20170106204620.GD29909@mtj.duckdns.org>

On Fri, Jan 6, 2017 at 10:46 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Dec 12, 2016 at 11:13:27PM +0530, Arvind Yadav wrote:
>> Here, If devm_ioremap will fail. It will return NULL.
>> Then hpriv->base = NULL - 0x20000; Kernel can run into
>> a NULL-pointer dereference. This error check will avoid
>> NULL pointer dereference.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> Applied to libata/for-4.10-fixes.

Hold on, why not to convert to devm_ioremap_resource() ?


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [LSF/MM TOPIC] status update on stream IDs
From: Andreas Dilger @ 2017-01-06 23:58 UTC (permalink / raw)
  To: lsf-pc; +Cc: linux-fsdevel, linux-block, linux-scsi, linux-nvme, linux-ide
In-Reply-To: <E86FC13D-0058-473C-B9D6-36B988ABEFFE@dilger.ca>

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

[resend to include other relevant lists]

On Jan 6, 2017, at 4:54 PM, Andreas Dilger <adilger@dilger.ca> wrote:
> 
> At LSF/MM'16 and Linux FAST (https://lwn.net/Articles/685499/) there were
> discussions about adding stream IDs to the block/device layer to allow higher
> layers (filesystems, applications) to identify IO streams so lower layers
> (SSDs, hybrid storage, etc.) can make better allocation/placement decisions.
> 
> It would be useful to get an update on the state of this work, and discuss
> any obstacles that need to be resolved for getting this code landed.


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* RE: [LSF/MM TOPIC] status update on stream IDs
From: Kwan (Hingkwan) Huen @ 2017-01-07  1:38 UTC (permalink / raw)
  To: Andreas Dilger, lsf-pc@lists.linux-foundation.org
  Cc: linux-fsdevel, linux-block@vger.kernel.org,
	linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org,
	linux-ide@vger.kernel.org
In-Reply-To: <7B0B6B53-D692-4084-AC63-55138132F509@dilger.ca>

It's been a while since Jens posted the write stream ID patches.

https://lwn.net/Articles/679136/

As recall, these patches provide the framework enabling applications to write data along with the stream ID hint. Writes with the same stream ID indicate that data being written related to each other. This simple ID hint allows SSD to organize data more efficiently internally, resulting better performance of the device in the long run and longer device lifespan. However, without a mature spec describing how the device works, and a device that can be easily acquired for testing, these patches were too early back then.

With the new NVMe v1.3 spec draft available now, which includes the latest streams directive feature, and we already have the device available on hand, would like to bring this up again discuss this with the community for opinions. Here are some ideas we have in mind based on what's been done in Jens' patch from the link above. Any comments and suggestions are welcome and appreciated.

Jens' patches already provides the frame work that passes the stream ID from Application to kernel with new system call streamid().  What's needed is that the block device driver will need to pick up the stream ID from the bio and attached to the write command to the device. This completes the whole data path.

The stream management (enable/allocate/open/close, etc.) can go through functions mapped via backing device info (bdi) to allow same function all from application to manage streams in either a SCSI or NVMe device. The actual functions will be implemented in the block device driver and stream status and statistics can be stored, accessed and updated in gendisk struct. 

If anyone wants kernel to assign the ID, the write requests will need to be intercepted to collect statistics of the workload pattern, run some algorithm that determines and applies the write requests to appropriate stream. Or Kernel maintainers would have already good idea for the stream ID assignment. In this case they can just implement it without adding extra stream detection modules. These can be done in the block device with stream mapping stored in gendisk struct.

Regards, 
kwan
________________________________________
From: Linux-nvme [linux-nvme-bounces@lists.infradead.org] on behalf of Andreas Dilger [adilger@dilger.ca]
Sent: Friday, January 06, 2017 3:58 PM
To: lsf-pc@lists.linux-foundation.org
Cc: linux-fsdevel; linux-block@vger.kernel.org; linux-nvme@lists.infradead.org; linux-scsi@vger.kernel.org; linux-ide@vger.kernel.org
Subject: Re: [LSF/MM TOPIC] status update on stream IDs

[resend to include other relevant lists]

On Jan 6, 2017, at 4:54 PM, Andreas Dilger <adilger@dilger.ca> wrote:
>
> At LSF/MM'16 and Linux FAST (https://lwn.net/Articles/685499/) there were
> discussions about adding stream IDs to the block/device layer to allow higher
> layers (filesystems, applications) to identify IO streams so lower layers
> (SSDs, hybrid storage, etc.) can make better allocation/placement decisions.
>
> It would be useful to get an update on the state of this work, and discuss
> any obstacles that need to be resolved for getting this code landed.


Cheers, Andreas






^ permalink raw reply

* RE: [LSF/MM TOPIC] status update on stream IDs
From: Changho Choi @ 2017-01-07  2:06 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: lsf-pc@lists.linux-foundation.org, linux-fsdevel@vger.kernel.org,
	linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
	linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org,
	Kwan (Hingkwan) Huen
In-Reply-To: <87B8B7C67ECB1048AE5FA3C64D62B73E1CA9473A@SSIEXCH-MB3.ssi.samsung.com>

Hi Andreas,

Thanks for the heads up!

There has been big progress on it. It was ratified as a NVMe 1.3 standard in Oct 31st, 2016 in NVMe org(As you may already understand it was ratified as SCSI standard in T10 in May, 2015).
The standard compliant product is available for NVMe and SAS in the market.
Also Jens Axboe implemented the features in Kernel, etc. 

We are more than happy to update the status in the meeting. 
Also it would be a great time to revive the Linux Kernel implementation discussion to support both application-assigned stream ID and kernel-assigned stream ID at the same time. 
I think people will start circulating the possible approaches sooner or later unless it is already started.

Thanks,
Changho


[resend to include other relevant lists]

On Jan 6, 2017, at 4:54 PM, Andreas Dilger <adilger@dilger.ca> wrote:
>
> At LSF/MM'16 and Linux FAST (https://lwn.net/Articles/685499/) there 
> were discussions about adding stream IDs to the block/device layer to 
> allow higher layers (filesystems, applications) to identify IO streams 
> so lower layers (SSDs, hybrid storage, etc.) can make better allocation/placement decisions.
>
> It would be useful to get an update on the state of this work, and 
> discuss any obstacles that need to be resolved for getting this code landed.


Cheers, Andreas






^ permalink raw reply

* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Christoph Hellwig @ 2017-01-08 10:08 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Christoph Hellwig, Tejun Heo, Geert Uytterhoeven, Michael Schmitz,
	linux-ide, linux-m68k, linux-kernel
In-Reply-To: <3484511.F6TxDiWTi4@amdc3058>

On Fri, Dec 30, 2016 at 06:14:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> * m32r: I don't know why it was restricted but it builds fine nowadays
>   (I will fix it later in incremental patch)
> 
> * s390: no PATA hardware, legacy IDE subsystem is also not supported

s390 now has PCIe support, so PATA could be attached through a PCIe to
PCI bridge in theory.  And while that is very theoretical not having
arch ifdefs is still something we should strive for, so I'd suggest
removing the condition entirely.

^ permalink raw reply

* Re: [V1] ata: sata_mv:- Handle return value of devm_ioremap.
From: Tejun Heo @ 2017-01-08 18:49 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Arvind Yadav, linux-ide, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vf79R8H89npEPjbWkWqPXEKAiJ4Yru98T5nYK0x+FZuxw@mail.gmail.com>

On Sat, Jan 07, 2017 at 12:52:57AM +0200, Andy Shevchenko wrote:
> On Fri, Jan 6, 2017 at 10:46 PM, Tejun Heo <tj@kernel.org> wrote:
> > On Mon, Dec 12, 2016 at 11:13:27PM +0530, Arvind Yadav wrote:
> >> Here, If devm_ioremap will fail. It will return NULL.
> >> Then hpriv->base = NULL - 0x20000; Kernel can run into
> >> a NULL-pointer dereference. This error check will avoid
> >> NULL pointer dereference.
> >>
> >> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> >
> > Applied to libata/for-4.10-fixes.
> 
> Hold on, why not to convert to devm_ioremap_resource() ?

Care to send the patch?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [V1] ata: sata_mv:- Handle return value of devm_ioremap.
From: Andy Shevchenko @ 2017-01-08 22:08 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Arvind Yadav, linux-ide, linux-kernel@vger.kernel.org
In-Reply-To: <20170108184922.GC9403@mtj.duckdns.org>

On Sun, Jan 8, 2017 at 8:49 PM, Tejun Heo <tj@kernel.org> wrote:
> On Sat, Jan 07, 2017 at 12:52:57AM +0200, Andy Shevchenko wrote:
>> On Fri, Jan 6, 2017 at 10:46 PM, Tejun Heo <tj@kernel.org> wrote:
>> > On Mon, Dec 12, 2016 at 11:13:27PM +0530, Arvind Yadav wrote:
>> >> Here, If devm_ioremap will fail. It will return NULL.
>> >> Then hpriv->base = NULL - 0x20000; Kernel can run into
>> >> a NULL-pointer dereference. This error check will avoid
>> >> NULL pointer dereference.
>> >>
>> >> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> >
>> > Applied to libata/for-4.10-fixes.
>>
>> Hold on, why not to convert to devm_ioremap_resource() ?
>
> Care to send the patch?

Incremental or substitute?


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH v1 1/1] ata: sata_mv: Convert to devm_ioremap_resource()
From: Andy Shevchenko @ 2017-01-08 22:49 UTC (permalink / raw)
  To: Tejun Heo, Arvind Yadav, linux-ide; +Cc: Andy Shevchenko

Convert to devm_ioremap_resource() which provides more consistent error
handling.

Note that devm_ioremap_resource() provides its own error messages.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ata/sata_mv.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 2f32782cea6d..58fe0b846fed 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4067,6 +4067,7 @@ static int mv_platform_probe(struct platform_device *pdev)
 	struct ata_host *host;
 	struct mv_host_priv *hpriv;
 	struct resource *res;
+	void __iomem *mmio;
 	int n_ports = 0, irq = 0;
 	int rc;
 	int port;
@@ -4085,8 +4086,9 @@ static int mv_platform_probe(struct platform_device *pdev)
 	 * Get the register base first
 	 */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (res == NULL)
-		return -EINVAL;
+	mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mmio))
+		return PTR_ERR(mmio);
 
 	/* allocate host */
 	if (pdev->dev.of_node) {
@@ -4130,12 +4132,7 @@ static int mv_platform_probe(struct platform_device *pdev)
 	hpriv->board_idx = chip_soc;
 
 	host->iomap = NULL;
-	hpriv->base = devm_ioremap(&pdev->dev, res->start,
-				   resource_size(res));
-	if (!hpriv->base)
-		return -ENOMEM;
-
-	hpriv->base -= SATAHC0_REG_BASE;
+	hpriv->base = mmio - SATAHC0_REG_BASE;
 
 	hpriv->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(hpriv->clk))
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH v1 1/1] ata: sata_mv: Convert to devm_ioremap_resource()
From: Tejun Heo @ 2017-01-09 12:43 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Arvind Yadav, linux-ide
In-Reply-To: <20170108224922.147261-1-andriy.shevchenko@linux.intel.com>

On Mon, Jan 09, 2017 at 12:49:22AM +0200, Andy Shevchenko wrote:
> Convert to devm_ioremap_resource() which provides more consistent error
> handling.
> 
> Note that devm_ioremap_resource() provides its own error messages.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to libata/for-4.11.

Thanks.

-- 
tejun

^ permalink raw reply

* LSF/MM 2017: Call for Proposals closes January 15th
From: Jeff Layton @ 2017-01-09 13:40 UTC (permalink / raw)
  To: linux-block, linux-btrfs, linux-cifs, linux-ext4, linux-fsdevel,
	linux-ide, linux-kernel, linux-mm, linux-nfs, linux-scsi,
	xfs@oss.sgi.com, ceph-devel, linux-nvme
  Cc: lsf-pc@lists.linux-foundation.org

We initially sent this pretty early this year, so this is a resend in           
case anyone missed the first posting. The call for topics and attendance        
requests is open until January 15th, 2017.

The original message follows:

----------------------8<------------------------

The annual Linux Storage, Filesystem and Memory Management (LSF/MM)
Summit for 2017 will be held on March 20th and 21st at the Hyatt
Cambridge, Cambridge, MA. LSF/MM is an invitation-only technical
workshop to map out improvements to the Linux storage, filesystem and
memory management subsystems that will make their way into the mainline
kernel within the coming years.

    http://events.linuxfoundation.org/events/linux-storage-filesystem-and-mm-summit

Like last year, LSF/MM will be colocated with the Linux Foundation Vault
conference which takes place on March 22nd and 23rd in the same Venue.
For those that do not know, Vault is designed to be an event where open
source storage and filesystem practitioners meet storage implementors
and, as such, it would be of benefit for LSF/MM attendees to attend.

Unlike past years, Vault admission is not free for LSF/MM attendees this
year unless they're giving a talk. There is a discount for LSF/MM
attendees, however we would also like to encourage folks to submit talk
proposals to speak at the Vault conference.

    http://events.linuxfoundation.org/events/vault

On behalf of the committee I am issuing a call for agenda proposals that
are suitable for cross-track discussion as well as technical subjects
for the breakout sessions.

If advance notice is required for visa applications then please point
that out in your proposal or request to attend, and submit the topic
as soon as possible.

1) Proposals for agenda topics should be sent before January 15th, 2016
to:

    lsf-pc@lists.linux-foundation.org

and cc the Linux list or lists that are relevant for the topic in
question:

    ATA:   linux-ide@vger.kernel.org
    Block: linux-block@vger.kernel.org
    FS:    linux-fsdevel@vger.kernel.org
    MM:    linux-mm@kvack.org
    SCSI:  linux-scsi@vger.kernel.org
    NVMe:  linux-nvme@lists.infradead.org

Please tag your proposal with [LSF/MM TOPIC] to make it easier to track.
In addition, please make sure to start a new thread for each topic
rather than following up to an existing one.  Agenda topics and
attendees will be selected by the program committee, but the final
agenda will be formed by consensus of the attendees on the day.

2) Requests to attend the summit for those that are not proposing a
topic should be sent to:

    lsf-pc@lists.linux-foundation.org

Please summarise what expertise you will bring to the meeting, and what
you would like to discuss. Please also tag your email with [LSF/MM
ATTEND] and send it as a new thread so there is less chance of it
getting lost.

We will try to cap attendance at around 25-30 per track to facilitate
discussions although the final numbers will depend on the room sizes at
the venue.

Brief presentations are allowed to guide discussion, but are strongly
discouraged. There will be no recording or audio bridge. However, we
expect that written minutes will be published as we did in previous
years:

2016: https://lwn.net/Articles/lsfmm2016/

2015: https://lwn.net/Articles/lsfmm2015/

2014: http://lwn.net/Articles/LSFMM2014/

2013: http://lwn.net/Articles/548089/

3) If you have feedback on last year's meeting that we can use to
improve this year's, please also send that to:

    lsf-pc@lists.linux-foundation.org

Thank you on behalf of the program committee:

Storage:
    James Bottomley
    Martin K. Petersen (track chair)
    Sagi Grimberg

Filesystems:
    Anna Schumaker
    Chris Mason
    Eric Sandeen
    Jan Kara
    Jeff Layton (summit chair)
    Josef Bacik (track chair)
    Trond Myklebust

MM:
    Johannes Weiner
    Rik van Riel (track chair)
-- 
Jeff Layton <jlayton@poochiereds.net>

^ permalink raw reply

* [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Geert Uytterhoeven @ 2017-01-09 14:49 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, linux-kernel, Geert Uytterhoeven

Replace the sparse 256-pointer array for looking up protocol strings by
a switch() statement to reduce kernel size.

According to bloat-o-meter, this saves 910 bytes on m68k (32-bit), and
1892 bytes on arm64 (64-bit).

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/ata/libata-eh.c | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 0e1ec37070d19b64..e7196fc29ff09434 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2606,21 +2606,39 @@ static void ata_eh_link_report(struct ata_link *link)
 				[DMA_TO_DEVICE]		= "out",
 				[DMA_FROM_DEVICE]	= "in",
 			};
-			static const char *prot_str[] = {
-				[ATA_PROT_UNKNOWN]	= "unknown",
-				[ATA_PROT_NODATA]	= "nodata",
-				[ATA_PROT_PIO]		= "pio",
-				[ATA_PROT_DMA]		= "dma",
-				[ATA_PROT_NCQ]		= "ncq dma",
-				[ATA_PROT_NCQ_NODATA]	= "ncq nodata",
-				[ATAPI_PROT_NODATA]	= "nodata",
-				[ATAPI_PROT_PIO]	= "pio",
-				[ATAPI_PROT_DMA]	= "dma",
-			};
+			const char *prot_str = NULL;
 
+			switch (qc->tf.protocol) {
+			case ATA_PROT_UNKNOWN:
+				prot_str = "unknown";
+				break;
+			case ATA_PROT_NODATA:
+				prot_str = "nodata";
+				break;
+			case ATA_PROT_PIO:
+				prot_str = "pio";
+				break;
+			case ATA_PROT_DMA:
+				prot_str = "dma";
+				break;
+			case ATA_PROT_NCQ:
+				prot_str = "ncq dma";
+				break;
+			case ATA_PROT_NCQ_NODATA:
+				prot_str = "ncq nodata";
+				break;
+			case ATAPI_PROT_NODATA:
+				prot_str = "nodata";
+				break;
+			case ATAPI_PROT_PIO:
+				prot_str = "pio";
+				break;
+			case ATAPI_PROT_DMA:
+				prot_str = "dma";
+				break;
+			}
 			snprintf(data_buf, sizeof(data_buf), " %s %u %s",
-				 prot_str[qc->tf.protocol], qc->nbytes,
-				 dma_str[qc->dma_dir]);
+				 prot_str, qc->nbytes, dma_str[qc->dma_dir]);
 		}
 
 		if (ata_is_atapi(qc->tf.protocol)) {
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Tejun Heo @ 2017-01-09 15:23 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-ide, linux-kernel
In-Reply-To: <1483973368-6828-1-git-send-email-geert@linux-m68k.org>

On Mon, Jan 09, 2017 at 03:49:28PM +0100, Geert Uytterhoeven wrote:
> Replace the sparse 256-pointer array for looking up protocol strings by
> a switch() statement to reduce kernel size.
> 
> According to bloat-o-meter, this saves 910 bytes on m68k (32-bit), and
> 1892 bytes on arm64 (64-bit).
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gees, thanks for catching that.

Applied to libata/for-4.11.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Geert Uytterhoeven @ 2017-01-09 15:40 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170109152319.GA12827@mtj.duckdns.org>

Hi Tejun,

On Mon, Jan 9, 2017 at 4:23 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Jan 09, 2017 at 03:49:28PM +0100, Geert Uytterhoeven wrote:
>> Replace the sparse 256-pointer array for looking up protocol strings by
>> a switch() statement to reduce kernel size.
>>
>> According to bloat-o-meter, this saves 910 bytes on m68k (32-bit), and
>> 1892 bytes on arm64 (64-bit).
>>
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Gees, thanks for catching that.
>
> Applied to libata/for-4.11.

Thanks!

There are two more that annoy me, but I don't know how to fix them:

ata_scsi_rbuf                                  -    4096   +4096
ata_force_param_buf                            -    4096   +4096

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Bartlomiej Zolnierkiewicz @ 2017-01-09 16:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide,
	linux-m68k, linux-kernel
In-Reply-To: <20170108100823.GD25268@infradead.org>


Hi,

On Sunday, January 08, 2017 02:08:23 AM Christoph Hellwig wrote:
> On Fri, Dec 30, 2016 at 06:14:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > * m32r: I don't know why it was restricted but it builds fine nowadays
> >   (I will fix it later in incremental patch)
> > 
> > * s390: no PATA hardware, legacy IDE subsystem is also not supported
> 
> s390 now has PCIe support, so PATA could be attached through a PCIe to
> PCI bridge in theory.  And while that is very theoretical not having
> arch ifdefs is still something we should strive for, so I'd suggest
> removing the condition entirely.

I agree.  I will do it later unless someone beats me to it.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Tejun Heo @ 2017-01-09 16:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdW6KPq6-7vmpTTv_M30zbqkdVbewgaOaCqYT+UzXpGU9Q@mail.gmail.com>

Hello,

On Mon, Jan 09, 2017 at 04:40:19PM +0100, Geert Uytterhoeven wrote:
> There are two more that annoy me, but I don't know how to fix them:
> 
> ata_scsi_rbuf                                  -    4096   +4096
> ata_force_param_buf                            -    4096   +4096

ata_force_param_buf is __initdata and shouldn't really matter.
ata_scsi_rbuf, hmmm, idk.  Maybe we can allocate it dynamically when
registering the first ATA device so that systems w/o them can avoid
the wastage.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Bartlomiej Zolnierkiewicz @ 2017-01-09 16:11 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tejun Heo, Michael Schmitz, linux-ide@vger.kernel.org, linux-m68k,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdVpnhTNSRZ1h-q8+d0wjDKSHtev0eyVoVxYWhf0DCD+zg@mail.gmail.com>


Hi,

On Tuesday, January 03, 2017 11:49:16 AM Geert Uytterhoeven wrote:
> Hi Bartlomiej,
> 
> On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > This patchset adds m68k/Atari Falcon PATA support to libata.
> 
> Thanks for your series!
> 
> That leaves us with 4 to go ;-)
> 
>     CONFIG_BLK_DEV_GAYLE
>     CONFIG_BLK_DEV_BUDDHA
>     CONFIG_BLK_DEV_MAC_IDE
>     CONFIG_BLK_DEV_Q40IDE

They should be easy to port to libata, one just needs to remember to
use ->qc_defer for ports that require serialization.

> Note that using libata instead of the legacy IDE driver increases kernel size.
> 
> After enabling libata:
> 
>     CONFIG_ATA=y
>     CONFIG_ATA_VERBOSE_ERROR=y
>     CONFIG_ATA_SFF=y
>     CONFIG_PATA_FALCON=y
> 
> an atari_defconfig kernel grew by:
> 
>     add/remove: 775/0 grow/shrink: 753/41 up/down: 98999/-242 (98757)
> 
> After disabling CONFIG_IDE:
> 
>     add/remove: 0/589 grow/shrink: 0/12 up/down: 0/-62835 (-62835)
> 
> So the net result is:
> 
>     add/remove: 775/589 grow/shrink: 749/51 up/down: 98886/-62964 (35922)

SATA-specific code is not needed on legacy PATA systems so it should be
possible to reduce the size by adding new config option (i.e. SATA_HOST) and:

- covering SATA only code with CONFIG_SATA_HOST ifdefs

- making SATA_HOST to be selected by SATA host drivers in drivers/ata/KConfig

> Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the
> value advertised by Kconfig (6KB).

If it is only ~1kB nowadays I would vote for making the error logging always
verbose and just removing CONFIG_ATA_VERBOSE_ERROR, Tejun?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply

* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Geert Uytterhoeven @ 2017-01-09 16:15 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Tejun Heo, Michael Schmitz, linux-ide@vger.kernel.org, linux-m68k,
	linux-kernel@vger.kernel.org
In-Reply-To: <1483106478-1382-2-git-send-email-b.zolnierkie@samsung.com>

On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> When libata was merged m68k lacked IOMAP support.  This has not been
> true for a long time now so allow subsystem to be used on m68k.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Christoph Hellwig @ 2017-01-09 16:21 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Geert Uytterhoeven, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170109160424.GB12827@mtj.duckdns.org>

On Mon, Jan 09, 2017 at 11:04:24AM -0500, Tejun Heo wrote:
> ata_force_param_buf is __initdata and shouldn't really matter.
> ata_scsi_rbuf, hmmm, idk.  Maybe we can allocate it dynamically when
> registering the first ATA device so that systems w/o them can avoid
> the wastage.

Having it global is kinda weird anyway.  But looking the code none
of the commands actually using is in the paging path, so it could
simply be replaced with a dynamic allocation in ata_scsi_rbuf_fill
for the actually needed size, which often will be very small,
or sometimes even 0.

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Geert Uytterhoeven @ 2017-01-09 16:30 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170109160424.GB12827@mtj.duckdns.org>

Hi Tejun,

On Mon, Jan 9, 2017 at 5:04 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Jan 09, 2017 at 04:40:19PM +0100, Geert Uytterhoeven wrote:
>> There are two more that annoy me, but I don't know how to fix them:
>>
>> ata_scsi_rbuf                                  -    4096   +4096
>> ata_force_param_buf                            -    4096   +4096
>
> ata_force_param_buf is __initdata and shouldn't really matter.

It mainly matters because of e.g. bootloader limitations.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Christoph Hellwig @ 2017-01-09 17:22 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Geert Uytterhoeven, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170109162143.GA11959@infradead.org>

On Mon, Jan 09, 2017 at 08:21:43AM -0800, Christoph Hellwig wrote:
> Having it global is kinda weird anyway.  But looking the code none
> of the commands actually using is in the paging path, so it could
> simply be replaced with a dynamic allocation in ata_scsi_rbuf_fill
> for the actually needed size, which often will be very small,
> or sometimes even 0.

Prototype here, only tested with a simple mkfs.xfs and some I/O on
AHCI so far:

http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/libata-kill-ata_scsi_rbuf

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Christoph Hellwig @ 2017-01-09 17:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tejun Heo, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdV3n9rq8eTjwxKzRLroF2YJYi2qvCzQ4eDhDEkGB=4dLw@mail.gmail.com>

On Mon, Jan 09, 2017 at 05:30:02PM +0100, Geert Uytterhoeven wrote:
> > ata_force_param_buf is __initdata and shouldn't really matter.
> 
> It mainly matters because of e.g. bootloader limitations.

Do we need a full 4k for the force parameters?  What would a typical
command line for it look like?

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Tejun Heo @ 2017-01-09 17:30 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170109172207.GA10557@infradead.org>

On Mon, Jan 09, 2017 at 09:22:07AM -0800, Christoph Hellwig wrote:
> Prototype here, only tested with a simple mkfs.xfs and some I/O on
> AHCI so far:
> 
> http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/libata-kill-ata_scsi_rbuf

Yeah, the only thing which needs completion from atomic context is
atapi commands.  If that can be done on stack, it's all good.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Tejun Heo @ 2017-01-09 17:31 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Geert Uytterhoeven, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170109172723.GA30423@infradead.org>

Hello,

On Mon, Jan 09, 2017 at 09:27:23AM -0800, Christoph Hellwig wrote:
> On Mon, Jan 09, 2017 at 05:30:02PM +0100, Geert Uytterhoeven wrote:
> > > ata_force_param_buf is __initdata and shouldn't really matter.
> > 
> > It mainly matters because of e.g. bootloader limitations.
> 
> Do we need a full 4k for the force parameters?  What would a typical
> command line for it look like?

Maybe a couple hundreds bytes at max, but it's a bit weird to restrict
this given that it is bss, not gigantic and __initdata.  What kind of
bootloader limitations are we talking about?

Thanks.

-- 
tejun

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox