* 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
* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Geert Uytterhoeven @ 2017-01-09 18:25 UTC (permalink / raw)
To: Tejun Heo
Cc: Christoph Hellwig, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170109173154.GE12827@mtj.duckdns.org>
On Mon, Jan 9, 2017 at 6:31 PM, Tejun Heo <tj@kernel.org> wrote:
> 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?
Some boot loaders start overwriting themselves or the passed DTB if the
kernel becomes too big.
If I'm not mistaken, bss is still expanded early (verified, increasing bss
can trigger the above problem).
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: Tejun Heo @ 2017-01-09 19:41 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Hellwig, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdXfFAyyT2VWwmG3sHu_Ufqzoz39O95rXDUCvHaD9HQFZA@mail.gmail.com>
Hello, Geert.
On Mon, Jan 09, 2017 at 07:25:31PM +0100, Geert Uytterhoeven wrote:
> On Mon, Jan 9, 2017 at 6:31 PM, Tejun Heo <tj@kernel.org> wrote:
> > 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?
>
> Some boot loaders start overwriting themselves or the passed DTB if the
> kernel becomes too big.
> If I'm not mistaken, bss is still expanded early (verified, increasing bss
> can trigger the above problem).
So, to avoid that, we can just kmalloc and kfree the buffer, but it
seems like a silly complication to work around bugs in some
bootloaders. There are many places in kernel where we're liberal
about __initdata which is great. I'm not sure complicating all those
places for a broken bootloader is a good idea.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: David Miller @ 2017-01-09 20:25 UTC (permalink / raw)
To: b.zolnierkie; +Cc: lramos.prof, linux-ide, petkovbb
In-Reply-To: <2007222.eymCZIUvPG@amdc3058>
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Fri, 30 Dec 2016 17:05:51 +0100
> Yes, it was a conscious decision.
I understand things now, I'll revert this change, thanks everyone.
^ permalink raw reply
* Re: [PATCH] libata-eh: Use switch() instead of sparse array for protocol strings
From: Geert Uytterhoeven @ 2017-01-09 20:28 UTC (permalink / raw)
To: Tejun Heo
Cc: Christoph Hellwig, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170109194100.GH12827@mtj.duckdns.org>
Hi Tejun,
On Mon, Jan 9, 2017 at 8:41 PM, Tejun Heo <tj@kernel.org> wrote:
> On Mon, Jan 09, 2017 at 07:25:31PM +0100, Geert Uytterhoeven wrote:
>> On Mon, Jan 9, 2017 at 6:31 PM, Tejun Heo <tj@kernel.org> wrote:
>> > 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?
>>
>> Some boot loaders start overwriting themselves or the passed DTB if the
>> kernel becomes too big.
>> If I'm not mistaken, bss is still expanded early (verified, increasing bss
>> can trigger the above problem).
>
> So, to avoid that, we can just kmalloc and kfree the buffer, but it
> seems like a silly complication to work around bugs in some
> bootloaders. There are many places in kernel where we're liberal
> about __initdata which is great. I'm not sure complicating all those
> places for a broken bootloader is a good idea.
Sure. We cannot avoid that kernels (esp. multiplatform) keep on growing.
But when I see a new 4KiB-sized buffer, i'm always suspicious...
A few years ago, I caught someone miscalculating shifts, leading
to a static buffer that was 256 times larger than intended ;-)
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: Tejun Heo @ 2017-01-09 20:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Christoph Hellwig, linux-ide@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdUVtQOJYfQsspSk9+v-4abGf+OTSMWEfzXvGLu9namqZg@mail.gmail.com>
Hello,
On Mon, Jan 09, 2017 at 09:28:12PM +0100, Geert Uytterhoeven wrote:
> > So, to avoid that, we can just kmalloc and kfree the buffer, but it
> > seems like a silly complication to work around bugs in some
> > bootloaders. There are many places in kernel where we're liberal
> > about __initdata which is great. I'm not sure complicating all those
> > places for a broken bootloader is a good idea.
>
> Sure. We cannot avoid that kernels (esp. multiplatform) keep on growing.
>
> But when I see a new 4KiB-sized buffer, i'm always suspicious...
> A few years ago, I caught someone miscalculating shifts, leading
> to a static buffer that was 256 times larger than intended ;-)
Oh, sure, things like the protocol string table are just stupid and
it's great that you caught it. I just don't think it makes sense to
scrutinize bss __initdata. It's not in kernel image and goes away
once the kernel boots. It's okay to a bit liberal with them for the
sake of simplicity.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 1/1] libata: Fix ATA request sense
From: Damien Le Moal @ 2017-01-10 0:54 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-ide, Hannes Reinecke
In-Reply-To: <20170106204416.GC29909@mtj.duckdns.org>
Tejun,
On 1/7/17 05:44, Tejun Heo wrote:
> 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.
Thank you.
Should we send this patch to stable too ?
This bug does break applications on kernels 4.7 to 4.9 (e.g. libzbc test
suite and many tools we have using SGIO to directly send ATA commands to
SATA disks). I suspect sg-utils and hdparm may be affected in some way too.
Best regards.
--
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com
^ permalink raw reply
* libata: remove the global response buffer
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
Avoid the need for a global 4k buffer by allocating no response buffer
for commands that don't return data, a small on-stack one for the ATAPI
fixup, or a kmalloced one for all other emulated commands.
^ permalink raw reply
* [PATCH 1/6] libata: avoid global response buffer in atapi_qc_complete
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
We only need to look at 4 bytes of the inquiry response for ATAPI
devices. Instead of using the global ata_scsi_rbuf just use a
a stack buffer. Also factor the fixup into it's own little helper
function to make it more readable.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 1f863e7..031a77a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2873,6 +2873,26 @@ static void atapi_request_sense(struct ata_queued_cmd *qc)
DPRINTK("EXIT\n");
}
+/*
+ * ATAPI devices typically report zero for their SCSI version, and sometimes
+ * deviate from the spec WRT response data format. If SCSI version is
+ * reported as zero like normal, then we make the following fixups:
+ * 1) Fake MMC-5 version, to indicate to the Linux scsi midlayer this is a
+ * modern device.
+ * 2) Ensure response data format / ATAPI information are always correct.
+ */
+static void atapi_fixup_inquiry(struct scsi_cmnd *cmd)
+{
+ u8 buf[4];
+
+ sg_copy_to_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
+ if (buf[2] == 0) {
+ buf[2] = 0x5;
+ buf[3] = 0x32;
+ }
+ sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, 4);
+}
+
static void atapi_qc_complete(struct ata_queued_cmd *qc)
{
struct scsi_cmnd *cmd = qc->scsicmd;
@@ -2927,30 +2947,8 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
*/
ata_gen_passthru_sense(qc);
} else {
- u8 *scsicmd = cmd->cmnd;
-
- if ((scsicmd[0] == INQUIRY) && ((scsicmd[1] & 0x03) == 0)) {
- unsigned long flags;
- u8 *buf;
-
- buf = ata_scsi_rbuf_get(cmd, true, &flags);
-
- /* ATAPI devices typically report zero for their SCSI version,
- * and sometimes deviate from the spec WRT response data
- * format. If SCSI version is reported as zero like normal,
- * then we make the following fixups: 1) Fake MMC-5 version,
- * to indicate to the Linux scsi midlayer this is a modern
- * device. 2) Ensure response data format / ATAPI information
- * are always correct.
- */
- if (buf[2] == 0) {
- buf[2] = 0x5;
- buf[3] = 0x32;
- }
-
- ata_scsi_rbuf_put(cmd, true, &flags);
- }
-
+ if (cmd->cmnd[0] == INQUIRY && (cmd->cmnd[1] & 0x03) == 0)
+ atapi_fixup_inquiry(cmd);
cmd->result = SAM_STAT_GOOD;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 2/6] libata: move struct ata_scsi_args to libata-scsi.c
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
It's only used in libata-scsi.c, so move it closer to the users.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 7 +++++++
drivers/ata/libata.h | 7 -------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 031a77a..43694f5 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2057,6 +2057,13 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
return SCSI_MLQUEUE_HOST_BUSY;
}
+struct ata_scsi_args {
+ struct ata_device *dev;
+ u16 *id;
+ struct scsi_cmnd *cmd;
+ void (*done)(struct scsi_cmnd *);
+};
+
/**
* ata_scsi_rbuf_get - Map response buffer.
* @cmd: SCSI command containing buffer to be mapped.
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 8f3a559..9943772 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -31,13 +31,6 @@
#define DRV_NAME "libata"
#define DRV_VERSION "3.00" /* must be exactly four chars */
-struct ata_scsi_args {
- struct ata_device *dev;
- u16 *id;
- struct scsi_cmnd *cmd;
- void (*done)(struct scsi_cmnd *);
-};
-
/* libata-core.c */
enum {
/* flags for ata_dev_read_id() */
--
2.1.4
^ permalink raw reply related
* [PATCH 3/6] libata: remove the done allback from ata_scsi_args
From: Christoph Hellwig @ 2017-01-10 8:41 UTC (permalink / raw)
To: tj; +Cc: geert, linux-ide
In-Reply-To: <1484037708-654-1-git-send-email-hch@lst.de>
It's always the scsi_done callback, and we can get at that easily
in the place where ->done is called.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/ata/libata-scsi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 43694f5..28e2530 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2061,7 +2061,6 @@ struct ata_scsi_args {
struct ata_device *dev;
u16 *id;
struct scsi_cmnd *cmd;
- void (*done)(struct scsi_cmnd *);
};
/**
@@ -2140,7 +2139,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
if (rc == 0)
cmd->result = SAM_STAT_GOOD;
- args->done(cmd);
+ cmd->scsi_done(cmd);
}
/**
@@ -4357,7 +4356,6 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
args.dev = dev;
args.id = dev->id;
args.cmd = cmd;
- args.done = cmd->scsi_done;
switch(scsicmd[0]) {
case INQUIRY:
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox